diff --git a/.ai/base.md b/.ai/base.md index 8084943e..da14ff78 100644 --- a/.ai/base.md +++ b/.ai/base.md @@ -44,6 +44,8 @@ Verify the real API before using or documenting it. Key packages: - `pkg/mutation/selectors/` — available container selectors - `pkg/feature/feature.go` — `NewVersionGate`, `Mutation[T]` - `pkg/recording/` — resource event recording +- `pkg/metrics/` — Prometheus implementation of `component.MetricsRecorder`: condition metrics plus the per-resource + apply counters - `pkg/testing/` — testing utilities (`golden/` for snapshot tests, `integration/` for integration helpers) When changing a public API, also check `examples/` for real usage patterns and to identify what else needs updating. diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 70278564..94b2e0e2 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -44,6 +44,8 @@ Verify the real API before using or documenting it. Key packages: - `pkg/mutation/selectors/` — available container selectors - `pkg/feature/feature.go` — `NewVersionGate`, `Mutation[T]` - `pkg/recording/` — resource event recording +- `pkg/metrics/` — Prometheus implementation of `component.MetricsRecorder`: condition metrics plus the per-resource + apply counters - `pkg/testing/` — testing utilities (`golden/` for snapshot tests, `integration/` for integration helpers) When changing a public API, also check `examples/` for real usage patterns and to identify what else needs updating. diff --git a/docs/cli.md b/docs/cli.md index eebbbf7e..2398fccc 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -90,15 +90,15 @@ package defaults to `certificate` (the lowercased kind), and the version default segment, which matches the API-version pattern). The four generated files: - `builder.go` registers the scaffolded default handlers, exposes the fluent configuration API (`WithMutation`, - `WithGuard`, `WithDataGuard`, `WithOptionalData`, the `WithCustom*` status setters), and `Build()` returns the - `Resource`. + `WithGuard`, `WithDataGuard`, `WithOptionalData`, `WithMetricsIdentifier`, the `WithCustom*` status setters), and + `Build()` returns the `Resource`. - `builder_test.go` tests `Build()` validation, that a registered mutation applies through the mutator, declared data extraction with `ExtractInto`, and `WithDataGuard`/`WithOptionalData` gating. - `mutator.go` defines `Mutator`, which records metadata and object edits and applies them in a single pass when `Apply()` runs. - `resource.go` defines `Resource`, which delegates every lifecycle method to the generic base: `Identity`, `Object`, - `Mutate`, the variant's status and suspension methods, `GuardStatus`, `ExtractData`, `ProducedData`, `ConsumedData`, - `RecordObservation`, `Preview`, `RegisteredMutations`, and `FiringSet`. + `Mutate`, `MetricsIdentifier`, the variant's status and suspension methods, `GuardStatus`, `ExtractData`, + `ProducedData`, `ConsumedData`, `RecordObservation`, `Preview`, `RegisteredMutations`, and `FiringSet`. Follow the printed next steps: run `go mod tidy` so `github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1` resolves in your module, then `go test ./certificate/...` to verify the generated package builds and passes before you diff --git a/docs/component.md b/docs/component.md index bd7c3a0a..9d21e7ae 100644 --- a/docs/component.md +++ b/docs/component.md @@ -646,7 +646,7 @@ recCtx := component.ReconcileContext{ Client: r.Client, // sigs.k8s.io/controller-runtime/pkg/client Scheme: r.Scheme, // *runtime.Scheme EventRecorder: r.EventRecorder, // events.EventRecorder, from manager.GetEventRecorder(name) - Metrics: r.Metrics, // component.MetricsRecorder (condition metrics), optional + Metrics: r.Metrics, // component.MetricsRecorder, optional; see Metrics APIReader: r.APIReader, // client.Reader, from manager.GetAPIReader(); direct reads on a status conflict Owner: owner, // the CRD that owns this component } @@ -655,9 +655,9 @@ err = comp.Reconcile(ctx, recCtx) ``` Dependencies are passed explicitly so components stay testable and decoupled from global state. The `Metrics` field is -optional; when set, the framework records Prometheus metrics for every condition reported during a reconcile, using the -recorder from [go-crd-condition-metrics](https://github.com/sourcehawk/go-crd-condition-metrics). Leave it `nil` to opt -out. +optional; when set, the framework records Prometheus metrics for every condition reported during a reconcile and for +every resource it applies. Use the recorder from `pkg/metrics`, or implement `component.MetricsRecorder` to record +elsewhere. Leave it `nil` to opt out. See [Metrics](#metrics). `EventRecorder` takes a `k8s.io/client-go/tools/events.EventRecorder`. The manager accessor that returns one, `GetEventRecorder(name)`, was added in controller-runtime v0.23; on v0.22.x, build the recorder from client-go instead, @@ -763,7 +763,7 @@ follows the server, and the next reconcile stages it again. source. After a successful update, `FlushStatus` records metrics for every condition on the owner. If `Metrics` is `nil`, -recording is skipped. +recording is skipped. See [Metrics](#metrics). This split is what lets a controller with several components stage several conditions during one reconcile and persist them in a single write. Persisting after each component would race the components' writes and produce 409 conflicts. See @@ -821,6 +821,91 @@ applies to a status-only controller too, as in the validation-only example above app.Status.ObservedGeneration = app.Generation ``` +## Metrics + +The framework records two families of Prometheus metrics through the single `ReconcileContext.Metrics` recorder: +**condition metrics**, one gauge per owner and condition type, and **resource metrics**, counters describing what the +framework did to each managed resource. + +`pkg/metrics` implements both. Build the collectors once per process and register them with controller-runtime's +registry, then give each controller its own recorder: + +```go +var ( + conditions = ocm.NewOperatorConditionsGauge("myoperator") + collectors = metrics.NewCollectors() +) + +func init() { + ctrlmetrics.Registry.MustRegister(conditions, collectors) +} + +recCtx := component.ReconcileContext{ + // ... + Metrics: metrics.NewRecorder("webapp-controller", conditions, collectors), +} +``` + +The controller name becomes the `controller` label on every series the recorder emits. Passing `nil` for either +collector disables that family; passing `nil` for `Metrics` itself disables both. + +### Resource metrics + +| Series | Type | Labels | +| --------------------------------- | ------- | ------------------------------------------------------------------------ | +| `ocf_resource_apply_total` | counter | `controller`, `owner_kind`, `component`, `resource`, `kind`, `operation` | +| `ocf_resource_apply_errors_total` | counter | `controller`, `owner_kind`, `component`, `resource`, `kind` | + +`operation` is `created`, `updated` or `none`, the same classification the framework reports through +[`ConvergingOperation`](#status-model) and the apply event. + +Both counters cover managed resources on the reconcile path and the suspension path. Read-only resources, deletions and +orphans are not applies and record nothing. + +Exactly one of the two counters moves per apply attempt, never both. The error counter covers every failure of the +attempt: mutating the desired object, the patch itself, and the classification that follows it. The one exception is a +failure so early that the framework has not worked out what it is applying, where no kind exists to label a series with; +those surface as an error condition on the owner instead. + +The reading that matters most is the `updated` rate. In steady state, a converged resource applies as `none` on every +pass and `updated` stays flat: + +```promql +rate(ocf_resource_apply_total{operation="updated"}[5m]) +``` + +A resource whose `updated` rate never settles to zero is being rewritten on every reconcile even though nothing changed. +Events report the same thing, but client-go's spam filter truncates them within seconds under exactly those conditions, +which is why the counter exists. + +### The resource identifier + +The `resource` label comes from `WithMetricsIdentifier` on the resource's builder: + +```go +secret.NewBuilder(tlsSecret). + WithMetricsIdentifier("tls"). + Build() +``` + +It is a Prometheus label value, not a name of anything in Kubernetes. When unset, the framework labels the resource with +its lowercased kind, so metrics work without any configuration. Set an identifier to tell two resources of the same kind +apart within one component, or when the object's name carries a generated suffix. `Build` rejects a blank identifier; +omit the call to accept the default. + +**The identifier must be low-cardinality and stable across reconciles**: a constant, or a value drawn from a small fixed +set. Deriving it from a per-owner value such as the owning custom resource's name creates one time series per owner, and +the framework never removes a series once created. + +Kept to that rule, the series set is bounded by the operator's static topology. No owner name or namespace appears in +these labels, so the same handful of series covers three owners or three thousand, and deleting a resource or an owner +leaves nothing behind that needs reaping. This is why there is no resource-metric counterpart to `RemoveConditionsFor`: +a resource that goes away simply stops incrementing, and deleting a counter series mid-flight would read downstream as a +counter reset and corrupt `rate()` and `increase()`. + +Per-owner detail lives in the condition metrics instead, which are keyed by owner name and namespace and are cleaned up +with `RemoveConditionsFor` when the owner is deleted. + ## Declared Data Resources inside one component pass observed values to each other through **data cells**. A cell is created in the diff --git a/docs/custom-resource.md b/docs/custom-resource.md index e9f7dadf..8e1a6d8f 100644 --- a/docs/custom-resource.md +++ b/docs/custom-resource.md @@ -715,6 +715,14 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the resource's identifier for resource-level metrics, +// used as the value of the `resource` label. It must be low-cardinality and stable +// across reconciles; when unset the framework labels the resource by kind. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // WithCustomConvergeStatus overrides the default convergence status handler. func (b *Builder) WithCustomConvergeStatus( handler func(concepts.ConvergingOperation, *examplev1.MessageQueue) (concepts.AliveStatusWithReason, error), @@ -890,6 +898,7 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) ``` !!! warning "Do not omit `Preview`" @@ -907,6 +916,11 @@ always if your builder exposes `ExtractInto`, `WithDataGuard`, or `WithOptionalD [build-time topology validation](component.md#build-time-validation) silently passes, `DataTopology()` omits the resource, and its cells are never cleared at the start of a reconcile. +Forward `MetricsIdentifier` whenever your builder exposes `WithMetricsIdentifier`. It satisfies +`concepts.MetricsIdentifiable`, which is how the framework reads the identifier at apply time. Without it the builder +accepts an identifier and the framework silently labels the resource by kind instead. See +[Metrics](component.md#metrics). + Forward `RecordObservation` whenever the resource may be registered read-only and declares an extraction. The framework feeds the fetched cluster object back to the resource before extraction runs; without it, the extraction would see the inert base passed to the builder rather than live cluster state. @@ -1162,10 +1176,11 @@ implications. ### Static resources Static resources have the simplest implementation. They do not participate in convergence, grace, or suspension -reporting. The builder uses `generic.NewStaticBuilder`, which supports `WithMutation`, `WithGuard`, `WithDataGuard`, and -`WithOptionalData`, plus a package-level `ExtractInto`. The resource wrapper needs only `Identity`, `Object`, `Mutate`, -`GuardStatus`, `ExtractData`, `ProducedData`, `ConsumedData`, `RecordObservation`, `Preview`, `RegisteredMutations`, and -`FiringSet`. `pkg/primitives/configmap` is a complete reference. +reporting. The builder uses `generic.NewStaticBuilder`, which supports `WithMutation`, `WithGuard`, `WithDataGuard`, +`WithOptionalData`, and `WithMetricsIdentifier`, plus a package-level `ExtractInto`. The resource wrapper needs only +`Identity`, `Object`, `Mutate`, `MetricsIdentifier`, `GuardStatus`, `ExtractData`, `ProducedData`, `ConsumedData`, +`RecordObservation`, `Preview`, `RegisteredMutations`, and `FiringSet`. `pkg/primitives/configmap` is a complete +reference. ### Task resources diff --git a/docs/getting-started.md b/docs/getting-started.md index a6d6759f..0a8e011d 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -271,7 +271,7 @@ self-induced update conflicts. | `Client` | `client.Client` | The controller-runtime client. | | `Scheme` | `*runtime.Scheme` | The operator scheme. | | `EventRecorder` | `events.EventRecorder` | For Kubernetes events. `GetEventRecorder(name)` on the manager, v0.23 and later. | -| `Metrics` | `component.MetricsRecorder` | Optional. Pass `nil` to skip status-condition metrics. | +| `Metrics` | `component.MetricsRecorder` | Optional. `metrics.NewRecorder(...)` records condition and resource metrics; pass `nil` to skip both. See [Metrics](component.md#metrics). | | `APIReader` | `client.Reader` | Optional, recommended. `GetAPIReader()` on the manager; `FlushStatus` reads through it on a 409 so the retry sees the live owner, not the informer cache. | | `Owner` | `component.OperatorCRD` | The owner object you fetched. Your CRD satisfies this via `GetStatusConditions`. | diff --git a/docs/primitives.md b/docs/primitives.md index ba356909..971bac1f 100644 --- a/docs/primitives.md +++ b/docs/primitives.md @@ -357,6 +357,26 @@ The interface deliberately omits operations that are not common to all three kin replica field), and the StatefulSet-only VolumeClaimTemplate methods. Reach for the concrete mutator type when you need those. +## Metrics Identifier + +Every primitive builder exposes `WithMetricsIdentifier`, which sets the value of the `resource` label on the framework's +per-resource apply counters: + +```go +res, err := secret.NewBuilder(tlsSecret). + WithMetricsIdentifier("tls"). + Build() +``` + +When unset, the framework labels the resource with its lowercased kind (`secret`, `deployment`), so the counters work +without configuration. Set an identifier to tell two resources of the same kind apart within one component, or when the +object's name carries a generated suffix. + +The identifier is a Prometheus label value, not a Kubernetes name, so it must be low-cardinality and stable across +reconciles. Never derive it from a per-owner value such as the owning custom resource's name. `Build` rejects a blank +identifier; omit the call to accept the default. See [Metrics](component.md#metrics) for the series, their labels, and +the cardinality contract. + ## Built-in Primitives | Primitive | Category | Documentation | diff --git a/e2e/component/suite_test.go b/e2e/component/suite_test.go index 49edd207..69f89e9b 100644 --- a/e2e/component/suite_test.go +++ b/e2e/component/suite_test.go @@ -9,6 +9,7 @@ import ( "github.com/sourcehawk/operator-component-framework/e2e/framework" ocm "github.com/sourcehawk/go-crd-condition-metrics/pkg/crd-condition-metrics" + "github.com/sourcehawk/operator-component-framework/pkg/metrics" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -55,15 +56,14 @@ var _ = BeforeSuite(func() { By("creating E2E reconciler") recorder := events.NewFakeRecorder(1000) - metrics := &ocm.ConditionMetricRecorder{ - Controller: "e2e-component", - OperatorConditionsGauge: ocm.NewOperatorConditionsGauge("e2e_component"), - } + metricsRecorder := metrics.NewRecorder( + "e2e-component", ocm.NewOperatorConditionsGauge("e2e_component"), metrics.NewCollectors(), + ) clusterReconciler = framework.NewClusterE2EReconciler( mgr.GetClient(), mgr.GetScheme(), recorder, - metrics, + metricsRecorder, mgr.GetAPIReader(), ) diff --git a/e2e/primitives/suite_test.go b/e2e/primitives/suite_test.go index 24bf8a38..a2bc0a2d 100644 --- a/e2e/primitives/suite_test.go +++ b/e2e/primitives/suite_test.go @@ -9,6 +9,7 @@ import ( "github.com/sourcehawk/operator-component-framework/e2e/framework" ocm "github.com/sourcehawk/go-crd-condition-metrics/pkg/crd-condition-metrics" + "github.com/sourcehawk/operator-component-framework/pkg/metrics" appsv1 "k8s.io/api/apps/v1" autoscalingv2 "k8s.io/api/autoscaling/v2" @@ -81,22 +82,21 @@ var _ = BeforeSuite(func() { By("creating E2E reconcilers") recorder := events.NewFakeRecorder(1000) - metrics := &ocm.ConditionMetricRecorder{ - Controller: "e2e-primitives", - OperatorConditionsGauge: ocm.NewOperatorConditionsGauge("e2e_primitives"), - } + metricsRecorder := metrics.NewRecorder( + "e2e-primitives", ocm.NewOperatorConditionsGauge("e2e_primitives"), metrics.NewCollectors(), + ) reconciler = framework.NewE2EReconciler( mgr.GetClient(), mgr.GetScheme(), recorder, - metrics, + metricsRecorder, mgr.GetAPIReader(), ) clusterReconciler = framework.NewClusterE2EReconciler( mgr.GetClient(), mgr.GetScheme(), recorder, - metrics, + metricsRecorder, mgr.GetAPIReader(), ) diff --git a/examples/component-prerequisites/main.go b/examples/component-prerequisites/main.go index ee1868b7..65d0b06c 100644 --- a/examples/component-prerequisites/main.go +++ b/examples/component-prerequisites/main.go @@ -14,6 +14,7 @@ import ( "github.com/sourcehawk/operator-component-framework/examples/component-prerequisites/app" "github.com/sourcehawk/operator-component-framework/examples/component-prerequisites/resources" sharedapp "github.com/sourcehawk/operator-component-framework/examples/shared/app" + "github.com/sourcehawk/operator-component-framework/pkg/metrics" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" @@ -47,13 +48,10 @@ func main() { gauge := ocm.NewOperatorConditionsGauge("example") controller := &app.Controller{ - Client: fakeClient, - Scheme: scheme, - EventRecorder: events.NewFakeRecorder(100), - Metrics: &ocm.ConditionMetricRecorder{ - Controller: "example", - OperatorConditionsGauge: gauge, - }, + Client: fakeClient, + Scheme: scheme, + EventRecorder: events.NewFakeRecorder(100), + Metrics: metrics.NewRecorder("example", gauge, metrics.NewCollectors()), NewConfigMapResource: resources.NewConfigMapResource, NewDeploymentResource: resources.NewDeploymentResource, } diff --git a/examples/custom-resource/README.md b/examples/custom-resource/README.md index cf3feeb7..f2e4567f 100644 --- a/examples/custom-resource/README.md +++ b/examples/custom-resource/README.md @@ -11,6 +11,8 @@ the **unstructured static builder**. `dnsNames`) using structured helpers rather than raw map manipulation. - **Metadata mutations**: `EditObjectMetadata` works the same way as on typed primitives. - **Declared extraction**: `static.ExtractInto` reads fields from the reconciled unstructured object into a data cell. +- **Resource metrics**: `metrics.NewRecorder` records condition metrics and per-resource apply counters, and + `WithMetricsIdentifier` keys the counters by a constant instead of the object's per-owner name. ## Use case @@ -22,6 +24,23 @@ mutation, and extraction patterns without writing a full typed primitive. 1. Create the CertificateRequest with DNS names and issuer reference. 2. Steady-state reconciliation. +3. Print the apply counters gathered from the registry. + +## Metrics + +The CertificateRequest is named `-cert`, so keying a metric by its name would create one time series per +`ExampleApp`, and the framework never removes a series once created. `WithMetricsIdentifier("certificate")` keys it by a +constant instead, and the series count stays fixed however many owners exist. + +The run ends by printing what the two reconciles did: + +```text +ocf_resource_apply_total{...,operation="created",resource="certificate"} 1 +ocf_resource_apply_total{...,operation="none",resource="certificate"} 1 +``` + +One create, then nothing to do. A resource whose `operation="updated"` count keeps climbing in steady state is being +rewritten on every reconcile even though nothing changed. See [Metrics](../../docs/component.md#metrics). ## Running diff --git a/examples/custom-resource/main.go b/examples/custom-resource/main.go index 0655b70d..517c812b 100644 --- a/examples/custom-resource/main.go +++ b/examples/custom-resource/main.go @@ -10,11 +10,14 @@ import ( "context" "fmt" "os" + "strings" + "github.com/prometheus/client_golang/prometheus" ocm "github.com/sourcehawk/go-crd-condition-metrics/pkg/crd-condition-metrics" "github.com/sourcehawk/operator-component-framework/examples/custom-resource/app" "github.com/sourcehawk/operator-component-framework/examples/custom-resource/resources" sharedapp "github.com/sourcehawk/operator-component-framework/examples/shared/app" + "github.com/sourcehawk/operator-component-framework/pkg/metrics" "k8s.io/apimachinery/pkg/api/meta" uns "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -57,15 +60,21 @@ func main() { exit("failed to create owner: %v", err) } + // Condition metrics and resource-level apply metrics are both recorded + // through one component.MetricsRecorder. Register the collectors once per + // process; a real operator would use controller-runtime's registry: + // + // ctrlmetrics.Registry.MustRegister(gauge, collectors) gauge := ocm.NewOperatorConditionsGauge("example") + collectors := metrics.NewCollectors() + registry := prometheus.NewRegistry() + registry.MustRegister(gauge, collectors) + controller := &app.Controller{ - Client: fakeClient, - Scheme: scheme, - EventRecorder: events.NewFakeRecorder(100), - Metrics: &ocm.ConditionMetricRecorder{ - Controller: "example", - OperatorConditionsGauge: gauge, - }, + Client: fakeClient, + Scheme: scheme, + EventRecorder: events.NewFakeRecorder(100), + Metrics: metrics.NewRecorder("example", gauge, collectors), NewCertificateResource: resources.NewCertificateResource, } @@ -83,6 +92,12 @@ func main() { } printConditions(owner) + // The apply counters record what each reconcile did to the resource: one + // created, then none. A resource whose "updated" count keeps climbing in + // steady state is being rewritten on every pass for no reason. + fmt.Println("\n--- Resource apply metrics ---") + printApplyMetrics(registry) + fmt.Println("\nDone.") } @@ -92,6 +107,28 @@ func printConditions(owner *app.ExampleApp) { } } +func printApplyMetrics(registry *prometheus.Registry) { + families, err := registry.Gather() + if err != nil { + exit("failed to gather metrics: %v", err) + } + for _, family := range families { + if family.GetName() != "ocf_resource_apply_total" { + continue + } + for _, metric := range family.GetMetric() { + labels := make([]string, 0, len(metric.GetLabel())) + for _, label := range metric.GetLabel() { + labels = append(labels, fmt.Sprintf("%s=%q", label.GetName(), label.GetValue())) + } + fmt.Printf( + " %s{%s} %g\n", + family.GetName(), strings.Join(labels, ","), metric.GetCounter().GetValue(), + ) + } + } +} + func mustAddToScheme(scheme *runtime.Scheme, fn func(*runtime.Scheme) error) { if err := fn(scheme); err != nil { exit("failed to add to scheme: %v", err) diff --git a/examples/custom-resource/resources/certificate.go b/examples/custom-resource/resources/certificate.go index dd3e935f..46982fb7 100644 --- a/examples/custom-resource/resources/certificate.go +++ b/examples/custom-resource/resources/certificate.go @@ -37,6 +37,12 @@ func BaseCertificateRequest(owner *app.ExampleApp) *uns.Unstructured { func NewCertificateResource(owner *app.ExampleApp) (component.Resource, error) { builder := static.NewBuilder(BaseCertificateRequest(owner)) + // The object's name embeds the owner's name, so it is the wrong thing to key + // a metric by: one time series per ExampleApp, and the framework never + // removes a series. The identifier is a constant instead, which keeps the + // series count fixed no matter how many owners exist. + builder.WithMetricsIdentifier("certificate") + builder.WithMutation(unstruct.Mutation{ Name: "certificate-spec", Mutate: func(m *unstruct.Mutator) error { diff --git a/examples/extraction-and-guards/main.go b/examples/extraction-and-guards/main.go index 5c833d92..38825046 100644 --- a/examples/extraction-and-guards/main.go +++ b/examples/extraction-and-guards/main.go @@ -15,6 +15,7 @@ import ( "github.com/sourcehawk/operator-component-framework/examples/extraction-and-guards/app" "github.com/sourcehawk/operator-component-framework/examples/extraction-and-guards/resources" sharedapp "github.com/sourcehawk/operator-component-framework/examples/shared/app" + "github.com/sourcehawk/operator-component-framework/pkg/metrics" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" @@ -46,13 +47,10 @@ func main() { gauge := ocm.NewOperatorConditionsGauge("example") controller := &app.Controller{ - Client: fakeClient, - Scheme: scheme, - EventRecorder: events.NewFakeRecorder(100), - Metrics: &ocm.ConditionMetricRecorder{ - Controller: "example", - OperatorConditionsGauge: gauge, - }, + Client: fakeClient, + Scheme: scheme, + EventRecorder: events.NewFakeRecorder(100), + Metrics: metrics.NewRecorder("example", gauge, metrics.NewCollectors()), NewConfigMapResource: resources.NewConfigMapResource, NewSecretResource: resources.NewSecretResource, } diff --git a/examples/grace-inconsistency/main.go b/examples/grace-inconsistency/main.go index 58518624..190bc0eb 100644 --- a/examples/grace-inconsistency/main.go +++ b/examples/grace-inconsistency/main.go @@ -15,6 +15,7 @@ import ( "github.com/sourcehawk/operator-component-framework/examples/grace-inconsistency/app" "github.com/sourcehawk/operator-component-framework/examples/grace-inconsistency/resources" sharedapp "github.com/sourcehawk/operator-component-framework/examples/shared/app" + "github.com/sourcehawk/operator-component-framework/pkg/metrics" appsv1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" @@ -45,13 +46,10 @@ func main() { gauge := ocm.NewOperatorConditionsGauge("example") controller := &app.Controller{ - Client: fakeClient, - Scheme: scheme, - EventRecorder: events.NewFakeRecorder(100), - Metrics: &ocm.ConditionMetricRecorder{ - Controller: "example", - OperatorConditionsGauge: gauge, - }, + Client: fakeClient, + Scheme: scheme, + EventRecorder: events.NewFakeRecorder(100), + Metrics: metrics.NewRecorder("example", gauge, metrics.NewCollectors()), NewDeploymentResource: resources.NewDeploymentResource, } diff --git a/examples/mutations-and-gating/main.go b/examples/mutations-and-gating/main.go index f41e1256..19f037de 100644 --- a/examples/mutations-and-gating/main.go +++ b/examples/mutations-and-gating/main.go @@ -14,6 +14,7 @@ import ( "github.com/sourcehawk/operator-component-framework/examples/mutations-and-gating/app" "github.com/sourcehawk/operator-component-framework/examples/mutations-and-gating/resources" sharedapp "github.com/sourcehawk/operator-component-framework/examples/shared/app" + "github.com/sourcehawk/operator-component-framework/pkg/metrics" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" @@ -51,13 +52,10 @@ func main() { gauge := ocm.NewOperatorConditionsGauge("example") controller := &app.Controller{ - Client: fakeClient, - Scheme: scheme, - EventRecorder: events.NewFakeRecorder(100), - Metrics: &ocm.ConditionMetricRecorder{ - Controller: "example", - OperatorConditionsGauge: gauge, - }, + Client: fakeClient, + Scheme: scheme, + EventRecorder: events.NewFakeRecorder(100), + Metrics: metrics.NewRecorder("example", gauge, metrics.NewCollectors()), NewDeploymentResource: resources.NewDeploymentResource, NewConfigMapResource: resources.NewConfigMapResource, } diff --git a/go.mod b/go.mod index 210d9338..fa7e1b64 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/onsi/ginkgo/v2 v2.32.0 github.com/onsi/gomega v1.42.1 github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 + github.com/prometheus/client_golang v1.23.2 github.com/sourcehawk/go-crd-condition-metrics v1.1.0 github.com/spf13/cobra v1.10.2 github.com/stretchr/testify v1.11.1 @@ -40,11 +41,11 @@ require ( github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect + github.com/kylelemons/godebug v1.1.0 // indirect github.com/mailru/easyjson v0.7.7 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect - github.com/prometheus/client_golang v1.23.2 // indirect github.com/prometheus/client_model v0.6.2 // indirect github.com/prometheus/common v0.67.5 // indirect github.com/prometheus/procfs v0.19.2 // indirect diff --git a/internal/scaffold/data.go b/internal/scaffold/data.go index e9aaa859..7b9600fd 100644 --- a/internal/scaffold/data.go +++ b/internal/scaffold/data.go @@ -1,6 +1,9 @@ package scaffold -import "fmt" +import ( + "fmt" + "strings" +) // TemplateData is the fully resolved input to the wrapper templates. Every field // is validated or derived by Options.Resolve. @@ -40,6 +43,13 @@ func (d TemplateData) PointerType() string { return "*" + d.QualifiedType() } +// LowercaseKind returns the kind in lower case, which is the value the +// framework uses for a resource's `resource` metric label when the wrapper's +// builder is not given an explicit metrics identifier. +func (d TemplateData) LowercaseKind() string { + return strings.ToLower(d.Kind) +} + // APIVersion returns "/", or bare "" for core types. func (d TemplateData) APIVersion() string { if d.Group == "" { diff --git a/internal/scaffold/templates/builder.go.tmpl b/internal/scaffold/templates/builder.go.tmpl index 0988c66a..8738c839 100644 --- a/internal/scaffold/templates/builder.go.tmpl +++ b/internal/scaffold/templates/builder.go.tmpl @@ -234,6 +234,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the {{.Kind}}'s identifier for resource-level metrics, +// used as the value of the `resource` label on ocf_resource_apply_total and +// ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be low-cardinality +// and stable across reconciles, never derived from a per-owner value such as the +// owning custom resource's name. When unset, the resource is labelled +// `{{.LowercaseKind}}`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/internal/scaffold/templates/resource.go.tmpl b/internal/scaffold/templates/resource.go.tmpl index b8e9177e..5305ecf0 100644 --- a/internal/scaffold/templates/resource.go.tmpl +++ b/internal/scaffold/templates/resource.go.tmpl @@ -39,6 +39,13 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with Builder.WithMetricsIdentifier, +// or an empty string when none was set, in which case the framework labels the +// resource with its lowercased kind. It satisfies concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying {{.Kind}} object. // // The returned object implements client.Object, making it compatible with @@ -169,3 +176,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/internal/scaffold/testdata/golden/integration/builder.go.golden b/internal/scaffold/testdata/golden/integration/builder.go.golden index a68bb57a..7fc5c701 100644 --- a/internal/scaffold/testdata/golden/integration/builder.go.golden +++ b/internal/scaffold/testdata/golden/integration/builder.go.golden @@ -202,6 +202,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the Ingress's identifier for resource-level metrics, +// used as the value of the `resource` label on ocf_resource_apply_total and +// ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be low-cardinality +// and stable across reconciles, never derived from a per-owner value such as the +// owning custom resource's name. When unset, the resource is labelled +// `ingress`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/internal/scaffold/testdata/golden/integration/resource.go.golden b/internal/scaffold/testdata/golden/integration/resource.go.golden index 315a713f..2a84148d 100644 --- a/internal/scaffold/testdata/golden/integration/resource.go.golden +++ b/internal/scaffold/testdata/golden/integration/resource.go.golden @@ -31,6 +31,13 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with Builder.WithMetricsIdentifier, +// or an empty string when none was set, in which case the framework labels the +// resource with its lowercased kind. It satisfies concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Ingress object. // // The returned object implements client.Object, making it compatible with @@ -153,3 +160,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/internal/scaffold/testdata/golden/static-cluster-scoped/builder.go.golden b/internal/scaffold/testdata/golden/static-cluster-scoped/builder.go.golden index 53764c82..8460bbba 100644 --- a/internal/scaffold/testdata/golden/static-cluster-scoped/builder.go.golden +++ b/internal/scaffold/testdata/golden/static-cluster-scoped/builder.go.golden @@ -86,6 +86,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the ClusterRole's identifier for resource-level metrics, +// used as the value of the `resource` label on ocf_resource_apply_total and +// ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be low-cardinality +// and stable across reconciles, never derived from a per-owner value such as the +// owning custom resource's name. When unset, the resource is labelled +// `clusterrole`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/internal/scaffold/testdata/golden/static-cluster-scoped/resource.go.golden b/internal/scaffold/testdata/golden/static-cluster-scoped/resource.go.golden index f1f40cea..4e27fc5d 100644 --- a/internal/scaffold/testdata/golden/static-cluster-scoped/resource.go.golden +++ b/internal/scaffold/testdata/golden/static-cluster-scoped/resource.go.golden @@ -28,6 +28,13 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with Builder.WithMetricsIdentifier, +// or an empty string when none was set, in which case the framework labels the +// resource with its lowercased kind. It satisfies concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying ClusterRole object. // // The returned object implements client.Object, making it compatible with @@ -109,3 +116,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/internal/scaffold/testdata/golden/static/builder.go.golden b/internal/scaffold/testdata/golden/static/builder.go.golden index f36dcbd8..efd7a826 100644 --- a/internal/scaffold/testdata/golden/static/builder.go.golden +++ b/internal/scaffold/testdata/golden/static/builder.go.golden @@ -84,6 +84,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the ConfigMap's identifier for resource-level metrics, +// used as the value of the `resource` label on ocf_resource_apply_total and +// ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be low-cardinality +// and stable across reconciles, never derived from a per-owner value such as the +// owning custom resource's name. When unset, the resource is labelled +// `configmap`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/internal/scaffold/testdata/golden/static/resource.go.golden b/internal/scaffold/testdata/golden/static/resource.go.golden index 9a933627..5d4ee445 100644 --- a/internal/scaffold/testdata/golden/static/resource.go.golden +++ b/internal/scaffold/testdata/golden/static/resource.go.golden @@ -28,6 +28,13 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with Builder.WithMetricsIdentifier, +// or an empty string when none was set, in which case the framework labels the +// resource with its lowercased kind. It satisfies concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying ConfigMap object. // // The returned object implements client.Object, making it compatible with @@ -109,3 +116,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/internal/scaffold/testdata/golden/task/builder.go.golden b/internal/scaffold/testdata/golden/task/builder.go.golden index c9ab9313..3cb5d154 100644 --- a/internal/scaffold/testdata/golden/task/builder.go.golden +++ b/internal/scaffold/testdata/golden/task/builder.go.golden @@ -176,6 +176,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the Job's identifier for resource-level metrics, +// used as the value of the `resource` label on ocf_resource_apply_total and +// ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be low-cardinality +// and stable across reconciles, never derived from a per-owner value such as the +// owning custom resource's name. When unset, the resource is labelled +// `job`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/internal/scaffold/testdata/golden/task/resource.go.golden b/internal/scaffold/testdata/golden/task/resource.go.golden index bd3b253e..f7529e79 100644 --- a/internal/scaffold/testdata/golden/task/resource.go.golden +++ b/internal/scaffold/testdata/golden/task/resource.go.golden @@ -30,6 +30,13 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with Builder.WithMetricsIdentifier, +// or an empty string when none was set, in which case the framework labels the +// resource with its lowercased kind. It satisfies concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Job object. // // The returned object implements client.Object, making it compatible with @@ -144,3 +151,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/internal/scaffold/testdata/golden/workload/builder.go.golden b/internal/scaffold/testdata/golden/workload/builder.go.golden index c9a44c61..44b20324 100644 --- a/internal/scaffold/testdata/golden/workload/builder.go.golden +++ b/internal/scaffold/testdata/golden/workload/builder.go.golden @@ -202,6 +202,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the Deployment's identifier for resource-level metrics, +// used as the value of the `resource` label on ocf_resource_apply_total and +// ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be low-cardinality +// and stable across reconciles, never derived from a per-owner value such as the +// owning custom resource's name. When unset, the resource is labelled +// `deployment`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/internal/scaffold/testdata/golden/workload/resource.go.golden b/internal/scaffold/testdata/golden/workload/resource.go.golden index f92b4169..9627958f 100644 --- a/internal/scaffold/testdata/golden/workload/resource.go.golden +++ b/internal/scaffold/testdata/golden/workload/resource.go.golden @@ -31,6 +31,13 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with Builder.WithMetricsIdentifier, +// or an empty string when none was set, in which case the framework labels the +// resource with its lowercased kind. It satisfies concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Deployment object. // // The returned object implements client.Object, making it compatible with @@ -153,3 +160,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/component/component.go b/pkg/component/component.go index 66104b2f..b7fd672d 100644 --- a/pkg/component/component.go +++ b/pkg/component/component.go @@ -33,9 +33,35 @@ type OperatorCRD interface { GetKind() string } -// MetricsRecorder is an interface for recording status condition changes as metrics. +// ResourceMetricLabels is the label set the framework attaches to +// resource-level metrics for a single managed resource. +// +// The recorder supplies the remaining `controller` label; the framework does +// not know which controller it is running inside. +type ResourceMetricLabels struct { + // OwnerKind is the kind of the custom resource that owns the component, + // recorded as the `owner_kind` label. + OwnerKind string + // Component is the name of the component that manages the resource, + // recorded as the `component` label. + Component string + // Identifier is the resource's stable metrics identifier, recorded as the + // `resource` label. It is the value set with WithMetricsIdentifier on the + // resource's builder, or the resource's lowercased kind when unset. + Identifier string + // Kind is the kind of the applied Kubernetes object, recorded as the + // `kind` label. + Kind string +} + +// MetricsRecorder records framework metrics: status condition changes and +// resource-level applies. +// // It is optional: a [ReconcileContext] may leave [ReconcileContext.Metrics] -// nil, in which case [FlushStatus] skips metric emission. +// nil, in which case [FlushStatus] and the apply path both skip emission. +// +// The metrics package ships an implementation backed by Prometheus. Implement +// the interface directly only to record somewhere else. type MetricsRecorder interface { // RecordConditionFor records a condition change for a specific object and kind. RecordConditionFor( @@ -43,6 +69,17 @@ type MetricsRecorder interface { conditionType, conditionStatus, conditionReason string, lastTransitionTime time.Time, extraLabelValues ...string, ) + // RecordResourceApply records one framework apply of a managed resource, + // classified by the operation the apply performed. It is called once per + // apply, on the reconcile path and the suspension path alike, and never for + // read-only resources. + RecordResourceApply(labels ResourceMetricLabels, operation concepts.ConvergingOperation) + // RecordResourceApplyError records one failed framework apply of a managed + // resource, covering every failure of the attempt: mutating the desired + // object, the patch itself, and the classification that follows it. Exactly + // one of RecordResourceApply and RecordResourceApplyError is called per + // attempt, never both. + RecordResourceApplyError(labels ResourceMetricLabels) } // ReconcileContext carries the dependencies and target object for a reconciliation loop. @@ -54,8 +91,9 @@ type ReconcileContext struct { // EventRecorder is the event recorder for publishing Kubernetes events. // Obtain one from the controller-runtime manager with GetEventRecorder(name). EventRecorder events.EventRecorder - // Metrics is the recorder for status condition metrics. It is optional; if - // nil, [FlushStatus] will skip metric emission. + // Metrics is the recorder for status condition metrics and resource-level + // apply metrics. It is optional; if nil, [FlushStatus] and the apply path + // both skip metric emission. Metrics MetricsRecorder // APIReader reads straight from the API server, bypassing the informer // cache. Obtain it from the controller-runtime manager with GetAPIReader(). diff --git a/pkg/component/component_test.go b/pkg/component/component_test.go index 8ee8eac7..b42cada7 100644 --- a/pkg/component/component_test.go +++ b/pkg/component/component_test.go @@ -273,6 +273,56 @@ var _ = Describe("Component Reconciler", func() { Expect(recorder.recordedWithReason("UpdatedConfigMap")).To(HaveLen(1)) }) + It("should stop counting updated applies once a rebuilt resource has converged", func() { + // Given: the same rebuilt-desired-object shape as above, which used to + // report Updated on every pass. Events were the only trace of that, and + // client-go's spam filter truncates them within seconds, so the metric + // is what makes it visible in a running cluster. + data := map[string]string{"foo": "bar"} + res := &operationRecordingResource{ + build: func() *corev1.ConfigMap { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "metered-cm", Namespace: namespace}, + Data: map[string]string{}, + } + for k, v := range data { + cm.Data[k] = v + } + return cm + }, + } + comp.reconcileResources = []reconcileEntry{ + {Resource: res, Options: resourceOptions{ParticipationMode: ParticipationModeRequired}}, + } + + // When: reconciled repeatedly without a change, then once after a change + for range 3 { + Expect(comp.Reconcile(ctx, recCtx)).To(Succeed()) + } + data["foo"] = "baz" + Expect(comp.Reconcile(ctx, recCtx)).To(Succeed()) + Expect(comp.Reconcile(ctx, recCtx)).To(Succeed()) + + // Then: none keeps growing while updated stays at its post-change value + spy := recCtx.Metrics.(*spyMetrics) + counts := map[concepts.ConvergingOperation]int{} + for _, apply := range spy.recordedApplies() { + counts[apply.operation]++ + } + Expect(counts[concepts.ConvergingOperationCreated]).To(Equal(1)) + Expect(counts[concepts.ConvergingOperationUpdated]).To(Equal(1)) + Expect(counts[concepts.ConvergingOperationNone]).To(Equal(3)) + Expect(spy.recordedErrors()).To(BeEmpty()) + + // And: every series carries the component, owner kind and kind default + Expect(spy.recordedApplies()[0].labels).To(Equal(ResourceMetricLabels{ + OwnerKind: owner.GetKind(), + Component: "test-component", + Identifier: "configmap", + Kind: "ConfigMap", + })) + }) + It("should classify a rebuilt typed CRD object and hand handlers exactly the server response", func() { // Given: a JSON-decoded typed object (CRDs are not served as protobuf, // and JSON decoding into a populated struct keeps fields the response diff --git a/pkg/component/concepts/metrics.go b/pkg/component/concepts/metrics.go new file mode 100644 index 00000000..cb6040c0 --- /dev/null +++ b/pkg/component/concepts/metrics.go @@ -0,0 +1,14 @@ +package concepts + +// MetricsIdentifiable is implemented by resources that carry a stable, +// low-cardinality identifier for resource-level metrics. +// +// The framework reads the identifier when it applies the resource and uses it +// as the value of the `resource` label. A resource that does not implement the +// interface, or that returns an empty string, is labelled with its lowercased +// kind instead. +type MetricsIdentifiable interface { + // MetricsIdentifier returns the resource's metrics identifier, or an empty + // string to accept the framework's default. + MetricsIdentifier() string +} diff --git a/pkg/component/conditions_test.go b/pkg/component/conditions_test.go index 44713d60..9801491e 100644 --- a/pkg/component/conditions_test.go +++ b/pkg/component/conditions_test.go @@ -34,6 +34,14 @@ func (m *MockMetrics) RecordConditionFor( m.Called(kind, object, conditionType, conditionStatus, conditionReason, lastTransitionTime, extraLabelValues) } +func (m *MockMetrics) RecordResourceApply(labels ResourceMetricLabels, operation concepts.ConvergingOperation) { + m.Called(labels, operation) +} + +func (m *MockMetrics) RecordResourceApplyError(labels ResourceMetricLabels) { + m.Called(labels) +} + func TestConvergingCondition(t *testing.T) { componentType := ConditionType("TestComponent") observedGen := int64(1) @@ -415,6 +423,14 @@ func (metricsThatPanic) RecordConditionFor(string, ocm.ObjectLike, string, strin panic("applyStatusCondition must not record metrics") } +func (metricsThatPanic) RecordResourceApply(ResourceMetricLabels, concepts.ConvergingOperation) { + panic("applyStatusCondition must not record metrics") +} + +func (metricsThatPanic) RecordResourceApplyError(ResourceMetricLabels) { + panic("applyStatusCondition must not record metrics") +} + // TestFlushStatusConflictOwnership covers the conflict path, where FlushStatus // must keep the staged owner as the object being written and take the server's // value only for condition types the framework does not own. diff --git a/pkg/component/create.go b/pkg/component/create.go index c0f0f94c..7a5899c5 100644 --- a/pkg/component/create.go +++ b/pkg/component/create.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "reflect" + "strings" "github.com/sourcehawk/operator-component-framework/internal/scope" "github.com/sourcehawk/operator-component-framework/pkg/component/concepts" @@ -25,7 +26,7 @@ import ( // reconcileResources (normal path) to avoid duplicating the Object/Mutate/SSA/ // status-collection sequence. func applyResource( - ctx context.Context, rec ReconcileContext, resource Resource, + ctx context.Context, rec ReconcileContext, resource Resource, componentName string, fieldOwner client.FieldOwner, mapper meta.RESTMapper, skipOwnerRef bool, ) (*reconcileResult, error) { obj, err := resource.Object() @@ -35,6 +36,20 @@ func applyResource( ) } + // Set GVK on the object (required for SSA — builders often omit TypeMeta). + // It runs this early so the object's kind is known for the metric labels on + // every later path, success and failure alike. + if err := ensureGVK(obj, rec.Scheme); err != nil { + return nil, fmt.Errorf( + "failed to determine GVK for resource %s: %w", resource.Identity(), err, + ) + } + + // Errors before this point leave no kind to label a metric with, and mean + // the framework never worked out what to apply. They surface as a returned + // error and an error condition instead. + labels := resourceMetricLabels(rec, componentName, resource, obj) + // Check if the object already exists (read through the client, usually the informer cache). // The observed object is fetched into a zeroed instance rather than a copy of // the desired object, so the pre-apply snapshot used by the comparison below @@ -43,24 +58,24 @@ func applyResource( var objectExists bool existing, err := newEmptyObjectLike(obj) if err != nil { - return nil, fmt.Errorf( + return nil, applyFailed(rec, labels, fmt.Errorf( "failed to prepare existence check for resource %s: %w", resource.Identity(), err, - ) + )) } if err := rec.Client.Get(ctx, client.ObjectKeyFromObject(obj), existing); err == nil { objectExists = true } else if !apierrors.IsNotFound(err) { - return nil, fmt.Errorf( + return nil, applyFailed(rec, labels, fmt.Errorf( "failed to check existence of resource %s: %w", resource.Identity(), err, - ) + )) } // Apply mutations to desired state ownerRefSkipped, err := mutateResource(resource, obj, rec.Owner, rec.Scheme, mapper, skipOwnerRef) if err != nil { - return nil, fmt.Errorf( + return nil, applyFailed(rec, labels, fmt.Errorf( "failed to mutate resource %s: %w", resource.Identity(), err, - ) + )) } // Prepare the object for SSA by clearing server-populated fields that must not @@ -69,20 +84,22 @@ func applyResource( // pointer and Patch writes back into the same object. clearServerFields(obj) - // Set GVK on the object (required for SSA — builders often omit TypeMeta) + // Re-assert the GVK. A Mutate implementation that assigns the whole struct + // (*current = *desired) drops the TypeMeta set above, and SSA requires it. + // The call is a no-op whenever the kind is still present. if err := ensureGVK(obj, rec.Scheme); err != nil { - return nil, fmt.Errorf( + return nil, applyFailed(rec, labels, fmt.Errorf( "failed to determine GVK for resource %s: %w", resource.Identity(), err, - ) + )) } // Server-Side Apply with forced ownership. // client.Apply is deprecated in favor of client.Client.Apply() which requires generated // ApplyConfiguration types. Using Patch with Apply is the pragmatic approach for untyped objects. if err := rec.Client.Patch(ctx, obj, client.Apply, client.ForceOwnership, fieldOwner); err != nil { //nolint:staticcheck - return nil, fmt.Errorf( + return nil, applyFailed(rec, labels, fmt.Errorf( "failed to apply resource %s: %w", resource.Identity(), err, - ) + )) } // Classify the apply by comparing the object observed before the patch (read @@ -95,9 +112,9 @@ func applyResource( if objectExists { changed, err := appliedObjectChanged(existing, obj) if err != nil { - return nil, fmt.Errorf( + return nil, applyFailed(rec, labels, fmt.Errorf( "failed to compare applied state of resource %s: %w", resource.Identity(), err, - ) + )) } convergingOperation = concepts.ConvergingOperationNone if changed { @@ -116,12 +133,15 @@ func applyResource( // Gather converging status of resources status, err := getConvergingStatus(resource, convergingOperation) if err != nil { - return nil, fmt.Errorf( + return nil, applyFailed(rec, labels, fmt.Errorf( "failed to determine converging status of resource %s: %w", resource.Identity(), err, - ) + )) } recording.RecordApplyOperationEvent(rec.EventRecorder, convergingOperation, obj, rec.Owner) + if rec.Metrics != nil { + rec.Metrics.RecordResourceApply(labels, convergingOperation) + } if status != nil { return &reconcileResult{Status: *status}, nil @@ -129,6 +149,48 @@ func applyResource( return nil, nil } +// resourceMetricLabels builds the label set for a resource's metrics. +// +// The identifier comes from the resource when it implements +// concepts.MetricsIdentifiable and returns a non-empty value, and defaults to +// the lowercased kind otherwise. The default is always bounded and needs no +// configuration, at the cost of collapsing two resources of the same kind in +// one component into a single series until one of them is given an identifier. +// +// The default is resolved here rather than in the resource, so that every +// Resource implementation is labelled the same way, hand-written ones included. +func resourceMetricLabels( + rec ReconcileContext, componentName string, resource Resource, obj client.Object, +) ResourceMetricLabels { + kind := obj.GetObjectKind().GroupVersionKind().Kind + identifier := strings.ToLower(kind) + if identifiable, ok := resource.(concepts.MetricsIdentifiable); ok { + // A blank identifier is treated as unset, matching what the builders + // reject at build time. A hand-written implementation is not held to + // that check, and " " is not a label value anyone meant to key a series + // by. Anything else is taken verbatim: rewriting a caller's identifier + // would silently split or merge series. + if configured := identifiable.MetricsIdentifier(); strings.TrimSpace(configured) != "" { + identifier = configured + } + } + return ResourceMetricLabels{ + OwnerKind: rec.Owner.GetKind(), + Component: componentName, + Identifier: identifier, + Kind: kind, + } +} + +// applyFailed records an apply error and returns the error unchanged, so that +// every error return after the object's kind is known stays a single statement. +func applyFailed(rec ReconcileContext, labels ResourceMetricLabels, err error) error { + if rec.Metrics != nil { + rec.Metrics.RecordResourceApplyError(labels) + } + return err +} + // applyResources ensures that all registered "creation" resources exist and match // the desired state in the Kubernetes cluster using Server-Side Apply. // @@ -161,7 +223,9 @@ func applyResources( var results []reconcileResult for _, entry := range entries { - result, err := applyResource(ctx, rec, entry.Resource, fieldOwner, mapper, entry.Options.Unowned) + result, err := applyResource( + ctx, rec, entry.Resource, componentName, fieldOwner, mapper, entry.Options.Unowned, + ) if err != nil { return nil, err } @@ -230,7 +294,9 @@ func reconcileResources( if entry.Options.ReadOnly { result, err = readResource(ctx, rec, resource) } else { - result, err = applyResource(ctx, rec, resource, fieldOwner, mapper, entry.Options.Unowned) + result, err = applyResource( + ctx, rec, resource, componentName, fieldOwner, mapper, entry.Options.Unowned, + ) } if err != nil { if entry.Options.ReadOnly && apierrors.IsNotFound(err) { diff --git a/pkg/component/create_metrics_test.go b/pkg/component/create_metrics_test.go new file mode 100644 index 00000000..1f7ddaee --- /dev/null +++ b/pkg/component/create_metrics_test.go @@ -0,0 +1,215 @@ +package component + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/sourcehawk/operator-component-framework/pkg/component/concepts" +) + +// identifiedResource is an operationRecordingResource that carries a metrics +// identifier, exercising the concepts.MetricsIdentifiable path. +type identifiedResource struct { + operationRecordingResource + identifier string +} + +func (r *identifiedResource) MetricsIdentifier() string { return r.identifier } + +// failingPatchClient fails every Patch and leaves every other operation intact, +// so an apply reaches the patch before it fails. +type failingPatchClient struct { + client.Client +} + +func (c failingPatchClient) Patch( + context.Context, client.Object, client.Patch, ...client.PatchOption, +) error { + return errors.New("patch rejected") +} + +// objectlessResource fails to produce an object at all, which happens before +// the framework can determine a kind to label metrics with. +type objectlessResource struct { + operationRecordingResource +} + +func (r *objectlessResource) Object() (client.Object, error) { + return nil, errors.New("cannot build object") +} + +func TestApplyResourceMetrics(t *testing.T) { + const namespace = "test-namespace" + + newEnv := func(t *testing.T) ReconcileContext { + t.Helper() + scheme := setupScheme() + owner := &MockOperatorCRD{ + ObjectMeta: metav1.ObjectMeta{Name: "test-owner", Namespace: namespace, UID: "owner-uid"}, + } + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme).WithObjects(owner).WithStatusSubresource(owner).Build() + return setupReconcileContext(scheme, owner, fakeClient) + } + + buildConfigMap := func() *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "rebuilt-cm", Namespace: namespace}, + Data: map[string]string{"foo": "bar"}, + } + } + + apply := func(t *testing.T, rec ReconcileContext, res Resource) error { + t.Helper() + _, err := applyResources( + t.Context(), rec, []reconcileEntry{{Resource: res}}, "test-component", createTestRESTMapper(), + ) + return err + } + + t.Run("records one apply per pass, defaulting the identifier to the lowercased kind", func(t *testing.T) { + rec := newEnv(t) + res := &operationRecordingResource{build: buildConfigMap} + + require.NoError(t, apply(t, rec, res)) + require.NoError(t, apply(t, rec, res)) + + applies := rec.Metrics.(*spyMetrics).recordedApplies() + require.Len(t, applies, 2) + assert.Equal(t, concepts.ConvergingOperationCreated, applies[0].operation) + assert.Equal(t, concepts.ConvergingOperationNone, applies[1].operation) + assert.Equal(t, ResourceMetricLabels{ + OwnerKind: "MockOperatorCRD", + Component: "test-component", + Identifier: "configmap", + Kind: "ConfigMap", + }, applies[0].labels) + assert.Empty(t, rec.Metrics.(*spyMetrics).recordedErrors()) + }) + + t.Run("uses the resource's identifier when it declares one", func(t *testing.T) { + rec := newEnv(t) + res := &identifiedResource{ + operationRecordingResource: operationRecordingResource{build: buildConfigMap}, + identifier: "tls", + } + + require.NoError(t, apply(t, rec, res)) + + applies := rec.Metrics.(*spyMetrics).recordedApplies() + require.Len(t, applies, 1) + assert.Equal(t, "tls", applies[0].labels.Identifier) + assert.Equal(t, "ConfigMap", applies[0].labels.Kind) + }) + + t.Run("falls back to the kind when the resource declares an empty identifier", func(t *testing.T) { + rec := newEnv(t) + res := &identifiedResource{ + operationRecordingResource: operationRecordingResource{build: buildConfigMap}, + identifier: "", + } + + require.NoError(t, apply(t, rec, res)) + + applies := rec.Metrics.(*spyMetrics).recordedApplies() + require.Len(t, applies, 1) + assert.Equal(t, "configmap", applies[0].labels.Identifier) + }) + + t.Run("falls back to the kind when the resource declares a whitespace-only identifier", func(t *testing.T) { + // The builders reject a blank identifier, but a hand-written + // concepts.MetricsIdentifiable can return one, and " " is not a label + // value anyone meant to key a series by. + rec := newEnv(t) + res := &identifiedResource{ + operationRecordingResource: operationRecordingResource{build: buildConfigMap}, + identifier: " \t ", + } + + require.NoError(t, apply(t, rec, res)) + + applies := rec.Metrics.(*spyMetrics).recordedApplies() + require.Len(t, applies, 1) + assert.Equal(t, "configmap", applies[0].labels.Identifier) + }) + + t.Run("records an error and no apply when the patch fails", func(t *testing.T) { + rec := newEnv(t) + rec.Client = failingPatchClient{Client: rec.Client} + res := &operationRecordingResource{build: buildConfigMap} + + require.Error(t, apply(t, rec, res)) + + spy := rec.Metrics.(*spyMetrics) + assert.Empty(t, spy.recordedApplies()) + assert.Equal(t, []ResourceMetricLabels{{ + OwnerKind: "MockOperatorCRD", + Component: "test-component", + Identifier: "configmap", + Kind: "ConfigMap", + }}, spy.recordedErrors()) + }) + + t.Run("records nothing when the object cannot be built, since no kind is known", func(t *testing.T) { + rec := newEnv(t) + res := &objectlessResource{} + + require.Error(t, apply(t, rec, res)) + + spy := rec.Metrics.(*spyMetrics) + assert.Empty(t, spy.recordedApplies()) + assert.Empty(t, spy.recordedErrors()) + }) + + t.Run("emits nothing when no recorder is configured", func(t *testing.T) { + rec := newEnv(t) + rec.Metrics = nil + res := &operationRecordingResource{build: buildConfigMap} + + assert.NotPanics(t, func() { require.NoError(t, apply(t, rec, res)) }) + }) + + t.Run("emits nothing for a read-only resource", func(t *testing.T) { + rec := newEnv(t) + require.NoError(t, rec.Client.Create(t.Context(), buildConfigMap())) + res := &observableConfigMapResource{name: "rebuilt-cm", namespace: namespace} + + _, err := reconcileResources( + t.Context(), rec, + []reconcileEntry{{Resource: res, Options: resourceOptions{ReadOnly: true}}}, + "test-component", createTestRESTMapper(), + ) + require.NoError(t, err) + + spy := rec.Metrics.(*spyMetrics) + assert.Empty(t, spy.recordedApplies()) + assert.Empty(t, spy.recordedErrors()) + }) +} + +// observableConfigMapResource is a read-only resource: the framework fetches it +// and never applies it, so it must never produce apply metrics. +type observableConfigMapResource struct { + name string + namespace string +} + +func (r *observableConfigMapResource) Identity() string { + return "v1/ConfigMap/" + r.namespace + "/" + r.name +} + +func (r *observableConfigMapResource) Object() (client.Object, error) { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: r.name, Namespace: r.namespace}, + }, nil +} + +func (r *observableConfigMapResource) Mutate(client.Object) error { return nil } diff --git a/pkg/component/create_test.go b/pkg/component/create_test.go index 46734d3e..e290bd74 100644 --- a/pkg/component/create_test.go +++ b/pkg/component/create_test.go @@ -548,6 +548,55 @@ func TestApplyResource_ConvergingOperation(t *testing.T) { }) } +// typeMetaClearingResource models a Mutate that assigns the whole struct, a +// realistic way to copy desired state onto the current object. That drops the +// TypeMeta the framework set, and Server-Side Apply requires it. +type typeMetaClearingResource struct { + namespace string +} + +func (r *typeMetaClearingResource) Identity() string { + return "v1/ConfigMap/" + r.namespace + "/typemeta-cm" +} + +func (r *typeMetaClearingResource) Object() (client.Object, error) { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "typemeta-cm", Namespace: r.namespace}, + }, nil +} + +func (r *typeMetaClearingResource) Mutate(obj client.Object) error { + cm, ok := obj.(*corev1.ConfigMap) + if !ok { + return fmt.Errorf("expected *corev1.ConfigMap, got %T", obj) + } + *cm = corev1.ConfigMap{ObjectMeta: cm.ObjectMeta, Data: map[string]string{"foo": "bar"}} + return nil +} + +func TestApplyResource_ReassertsGVKAfterMutate(t *testing.T) { + const namespace = "test-namespace" + scheme := setupScheme() + owner := &MockOperatorCRD{ + ObjectMeta: metav1.ObjectMeta{Name: "test-owner", Namespace: namespace, UID: "owner-uid"}, + } + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme).WithObjects(owner).WithStatusSubresource(owner).Build() + rec := setupReconcileContext(scheme, owner, fakeClient) + + res := &typeMetaClearingResource{namespace: namespace} + _, err := applyResources( + t.Context(), rec, []reconcileEntry{{Resource: res}}, "test-component", createTestRESTMapper(), + ) + require.NoError(t, err) + + applied := &corev1.ConfigMap{} + require.NoError(t, fakeClient.Get( + t.Context(), client.ObjectKey{Name: "typemeta-cm", Namespace: namespace}, applied, + )) + assert.Equal(t, map[string]string{"foo": "bar"}, applied.Data) +} + func TestNewEmptyObjectLike(t *testing.T) { t.Run("returns a zeroed typed object of the same type", func(t *testing.T) { src := &corev1.ConfigMap{ diff --git a/pkg/component/suite_test.go b/pkg/component/suite_test.go index 39e90025..a8d15a42 100644 --- a/pkg/component/suite_test.go +++ b/pkg/component/suite_test.go @@ -16,8 +16,6 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" - ocm "github.com/sourcehawk/go-crd-condition-metrics/pkg/crd-condition-metrics" - . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -94,10 +92,7 @@ func newTestReconcileContext(owner OperatorCRD) ReconcileContext { Client: k8sClient, Scheme: scheme.Scheme, EventRecorder: &spyRecorder{}, - Metrics: &ocm.ConditionMetricRecorder{ - Controller: "test-controller", - OperatorConditionsGauge: ocm.NewOperatorConditionsGauge("test_namespace"), - }, - Owner: owner, + Metrics: &spyMetrics{}, + Owner: owner, } } diff --git a/pkg/component/zz_setup_test.go b/pkg/component/zz_setup_test.go index e541b57c..179962a7 100644 --- a/pkg/component/zz_setup_test.go +++ b/pkg/component/zz_setup_test.go @@ -3,6 +3,7 @@ package component import ( "fmt" "sync" + "time" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" @@ -11,6 +12,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ocm "github.com/sourcehawk/go-crd-condition-metrics/pkg/crd-condition-metrics" + + "github.com/sourcehawk/operator-component-framework/pkg/component/concepts" ) // recordedEvent captures every argument handed to [events.EventRecorder.Eventf]. @@ -78,10 +81,54 @@ func setupReconcileContext(scheme *runtime.Scheme, owner *MockOperatorCRD, clien Client: client, Scheme: scheme, EventRecorder: &spyRecorder{}, - Metrics: &ocm.ConditionMetricRecorder{ - Controller: "test-controller", - OperatorConditionsGauge: ocm.NewOperatorConditionsGauge("test_namespace"), - }, - Owner: owner, + Metrics: &spyMetrics{}, + Owner: owner, } } + +// recordedApply captures one call to [MetricsRecorder.RecordResourceApply]. +type recordedApply struct { + labels ResourceMetricLabels + operation concepts.ConvergingOperation +} + +// spyMetrics is a [MetricsRecorder] that captures resource-level emissions in +// memory. Condition recording is exercised separately through MockMetrics. +type spyMetrics struct { + mu sync.Mutex + applies []recordedApply + errors []ResourceMetricLabels +} + +var _ MetricsRecorder = &spyMetrics{} + +func (s *spyMetrics) RecordConditionFor( + string, ocm.ObjectLike, string, string, string, time.Time, ...string, +) { +} + +func (s *spyMetrics) RecordResourceApply(labels ResourceMetricLabels, operation concepts.ConvergingOperation) { + s.mu.Lock() + defer s.mu.Unlock() + s.applies = append(s.applies, recordedApply{labels: labels, operation: operation}) +} + +func (s *spyMetrics) RecordResourceApplyError(labels ResourceMetricLabels) { + s.mu.Lock() + defer s.mu.Unlock() + s.errors = append(s.errors, labels) +} + +// recordedApplies returns a copy of the applies captured so far. +func (s *spyMetrics) recordedApplies() []recordedApply { + s.mu.Lock() + defer s.mu.Unlock() + return append([]recordedApply(nil), s.applies...) +} + +// recordedErrors returns a copy of the apply errors captured so far. +func (s *spyMetrics) recordedErrors() []ResourceMetricLabels { + s.mu.Lock() + defer s.mu.Unlock() + return append([]ResourceMetricLabels(nil), s.errors...) +} diff --git a/pkg/generic/builder_base.go b/pkg/generic/builder_base.go index 3c63434e..2520bd30 100644 --- a/pkg/generic/builder_base.go +++ b/pkg/generic/builder_base.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "reflect" + "strings" "github.com/sourcehawk/operator-component-framework/pkg/component/concepts" "sigs.k8s.io/controller-runtime/pkg/client" @@ -26,6 +27,11 @@ func isNil(i any) bool { type BaseBuilder[T client.Object, M FeatureMutator] struct { BaseRes *BaseResource[T, M] clusterScope bool + + // metricsIdentifierSet records that WithMetricsIdentifier was called, so + // ValidateBase can tell a blank identifier (a mistake) from an omitted one + // (a request for the default). Both leave the resource's identifier empty. + metricsIdentifierSet bool } // InitBase initializes the base resource configuration. @@ -64,6 +70,27 @@ func (b *BaseBuilder[T, M]) WithMutation(ms ...Mutation[M]) { b.BaseRes.Mutations = append(b.BaseRes.Mutations, ms...) } +// WithMetricsIdentifier sets the resource's identifier for resource-level +// metrics. It becomes the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// This is a Prometheus label value, not the name of anything in Kubernetes. It +// must be low-cardinality and stable across reconciles: a constant, or a value +// drawn from a small fixed set. Deriving it from a per-owner value such as the +// custom resource's name produces one time series per owner, and the framework +// never removes a series once created. +// +// When unset, the framework labels the resource with its lowercased kind. Set +// an identifier to tell two resources of the same kind apart within one +// component, or when the object's name carries a generated suffix. +// +// The identifier must not be blank. Build rejects an empty or whitespace-only +// value; omit the call to accept the default. +func (b *BaseBuilder[T, M]) WithMetricsIdentifier(identifier string) { + b.BaseRes.metricsIdentifier = identifier + b.metricsIdentifierSet = true +} + // WithGuard registers a guard precondition for the resource. // The guard is evaluated before each apply during reconciliation. If it returns // Blocked, the resource and all resources after it are skipped until the guard clears. @@ -178,6 +205,13 @@ func (b *BaseBuilder[T, M]) ValidateBase() error { return errors.New("identity function cannot be nil") } + // A blank identifier is a mistake rather than a request for the default: + // omitting the call is how the default is requested. It would otherwise + // produce an empty `resource` label that reads as a framework bug. + if b.metricsIdentifierSet && strings.TrimSpace(b.BaseRes.metricsIdentifier) == "" { + return errors.New("metrics identifier cannot be blank") + } + if b.BaseRes.NewMutator == nil { return errors.New("mutator factory cannot be nil") } diff --git a/pkg/generic/builder_integration.go b/pkg/generic/builder_integration.go index c645d646..8f6898c5 100644 --- a/pkg/generic/builder_integration.go +++ b/pkg/generic/builder_integration.go @@ -51,6 +51,13 @@ func (b *IntegrationBuilder[T, M]) WithMutation( return b } +// WithMetricsIdentifier sets the integration resource's metrics identifier. See +// BaseBuilder.WithMetricsIdentifier. +func (b *IntegrationBuilder[T, M]) WithMetricsIdentifier(identifier string) *IntegrationBuilder[T, M] { + b.BaseBuilder.WithMetricsIdentifier(identifier) + return b +} + // WithGuard registers a guard precondition for the integration resource. func (b *IntegrationBuilder[T, M]) WithGuard( handler func(T) (concepts.GuardStatusWithReason, error), diff --git a/pkg/generic/builder_static.go b/pkg/generic/builder_static.go index a71082e6..883273b4 100644 --- a/pkg/generic/builder_static.go +++ b/pkg/generic/builder_static.go @@ -42,6 +42,13 @@ func (b *StaticBuilder[T, M]) WithMutation(ms ...Mutation[M]) *StaticBuilder[T, return b } +// WithMetricsIdentifier sets the static resource's metrics identifier. See +// BaseBuilder.WithMetricsIdentifier. +func (b *StaticBuilder[T, M]) WithMetricsIdentifier(identifier string) *StaticBuilder[T, M] { + b.BaseBuilder.WithMetricsIdentifier(identifier) + return b +} + // WithGuard registers a guard precondition for the static resource. func (b *StaticBuilder[T, M]) WithGuard( handler func(T) (concepts.GuardStatusWithReason, error), diff --git a/pkg/generic/builder_task.go b/pkg/generic/builder_task.go index 9458e367..3cbc01ee 100644 --- a/pkg/generic/builder_task.go +++ b/pkg/generic/builder_task.go @@ -44,6 +44,13 @@ func (b *TaskBuilder[T, M]) WithMutation( return b } +// WithMetricsIdentifier sets the task resource's metrics identifier. See +// BaseBuilder.WithMetricsIdentifier. +func (b *TaskBuilder[T, M]) WithMetricsIdentifier(identifier string) *TaskBuilder[T, M] { + b.BaseBuilder.WithMetricsIdentifier(identifier) + return b +} + // WithGuard registers a guard precondition for the task resource. func (b *TaskBuilder[T, M]) WithGuard( handler func(T) (concepts.GuardStatusWithReason, error), diff --git a/pkg/generic/builder_workload.go b/pkg/generic/builder_workload.go index bd26cb07..829be019 100644 --- a/pkg/generic/builder_workload.go +++ b/pkg/generic/builder_workload.go @@ -54,6 +54,13 @@ func (b *WorkloadBuilder[T, M]) WithMutation( return b } +// WithMetricsIdentifier sets the workload resource's metrics identifier. See +// BaseBuilder.WithMetricsIdentifier. +func (b *WorkloadBuilder[T, M]) WithMetricsIdentifier(identifier string) *WorkloadBuilder[T, M] { + b.BaseBuilder.WithMetricsIdentifier(identifier) + return b +} + // WithGuard registers a guard precondition for the workload resource. func (b *WorkloadBuilder[T, M]) WithGuard( handler func(T) (concepts.GuardStatusWithReason, error), diff --git a/pkg/generic/metrics_identifier_test.go b/pkg/generic/metrics_identifier_test.go new file mode 100644 index 00000000..9f82c3c1 --- /dev/null +++ b/pkg/generic/metrics_identifier_test.go @@ -0,0 +1,53 @@ +package generic + +import ( + "strconv" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/sourcehawk/operator-component-framework/pkg/component/concepts" +) + +// newMetricsTestBuilder returns a valid static builder for a ConfigMap, used to +// exercise the metrics identifier without repeating the boilerplate. +func newMetricsTestBuilder() *StaticBuilder[*corev1.ConfigMap, *mockMutator] { + return NewStaticBuilder[*corev1.ConfigMap, *mockMutator]( + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "web-tls", Namespace: "default"}}, + func(cm *corev1.ConfigMap) string { return "v1/ConfigMap/" + cm.Namespace + "/" + cm.Name }, + func(*corev1.ConfigMap) *mockMutator { return &mockMutator{} }, + ) +} + +func TestBaseResourceMetricsIdentifier(t *testing.T) { + t.Run("returns empty when unset so the framework applies its default", func(t *testing.T) { + res, err := newMetricsTestBuilder().Build() + require.NoError(t, err) + assert.Empty(t, res.MetricsIdentifier()) + }) + + t.Run("returns the configured identifier", func(t *testing.T) { + res, err := newMetricsTestBuilder().WithMetricsIdentifier("tls").Build() + require.NoError(t, err) + assert.Equal(t, "tls", res.MetricsIdentifier()) + }) + + t.Run("satisfies concepts.MetricsIdentifiable", func(t *testing.T) { + res, err := newMetricsTestBuilder().Build() + require.NoError(t, err) + var identifiable concepts.MetricsIdentifiable = res + assert.Empty(t, identifiable.MetricsIdentifier()) + }) +} + +func TestBaseBuilderRejectsBlankMetricsIdentifier(t *testing.T) { + for _, identifier := range []string{"", " ", "\t "} { + t.Run("rejects "+strconv.Quote(identifier), func(t *testing.T) { + _, err := newMetricsTestBuilder().WithMetricsIdentifier(identifier).Build() + assert.EqualError(t, err, "metrics identifier cannot be blank") + }) + } +} diff --git a/pkg/generic/resource_base.go b/pkg/generic/resource_base.go index bcdb37e8..db9a733f 100644 --- a/pkg/generic/resource_base.go +++ b/pkg/generic/resource_base.go @@ -15,6 +15,12 @@ type BaseResource[T client.Object, M FeatureMutator] struct { IdentityFunc func(T) string + // metricsIdentifier is the value the framework uses for the `resource` label + // on resource-level metrics. An empty value means the framework's default + // applies. It is set through BaseBuilder.WithMetricsIdentifier, which is the + // only supported way to configure it, and read back with MetricsIdentifier. + metricsIdentifier string + // DataExtractions holds the declared data extractions recorded by // ExtractInto, run by ExtractData after the resource is applied or fetched. DataExtractions []DataExtraction[T] @@ -40,6 +46,13 @@ func (r *BaseResource[T, M]) Identity() string { return r.IdentityFunc(r.DesiredObject) } +// MetricsIdentifier returns the resource's configured metrics identifier, or an +// empty string when none was set, in which case the framework labels the +// resource with its lowercased kind. It satisfies concepts.MetricsIdentifiable. +func (r *BaseResource[T, M]) MetricsIdentifier() string { + return r.metricsIdentifier +} + // RegisteredMutations returns the deduplicated Names of every mutation registered // on the resource, in registration order, independent of the version it was built // at. It satisfies concepts.MutationInspector. diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go new file mode 100644 index 00000000..b316d6f4 --- /dev/null +++ b/pkg/metrics/metrics.go @@ -0,0 +1,187 @@ +// Package metrics provides the framework's Prometheus implementation of +// [component.MetricsRecorder]. +// +// It records two things: the status condition metrics of +// [go-crd-condition-metrics], and the framework's own resource-level apply +// counters. Wire one [Recorder] per controller and share a single [Collectors] +// across the process: +// +// var ( +// conditions = ocm.NewOperatorConditionsGauge("myoperator") +// collectors = metrics.NewCollectors() +// ) +// +// func init() { +// ctrlmetrics.Registry.MustRegister(conditions, collectors) +// } +// +// rec := component.ReconcileContext{ +// // ... +// Metrics: metrics.NewRecorder("webapp-controller", conditions, collectors), +// } +// +// [go-crd-condition-metrics]: https://github.com/sourcehawk/go-crd-condition-metrics +package metrics + +import ( + "strings" + "time" + + "github.com/prometheus/client_golang/prometheus" + ocm "github.com/sourcehawk/go-crd-condition-metrics/pkg/crd-condition-metrics" + + "github.com/sourcehawk/operator-component-framework/pkg/component" + "github.com/sourcehawk/operator-component-framework/pkg/component/concepts" +) + +// Label names of the resource-level metrics, in the order the counters declare +// them. The `controller` label separates operators, and controllers within an +// operator, that share one registry. +var ( + applyLabels = []string{"controller", "owner_kind", "component", "resource", "kind", "operation"} + errorLabels = []string{"controller", "owner_kind", "component", "resource", "kind"} +) + +// Collectors holds the framework's resource-level Prometheus collectors: +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// Construct one per process and register it once, before any Recorder that +// uses it starts reconciling: +// +// collectors := metrics.NewCollectors() +// ctrlmetrics.Registry.MustRegister(collectors) +// +// Every controller in the process then shares it, told apart by the +// `controller` label their Recorder supplies. +// +// The series are keyed only by the operator's static topology: controller, +// owner kind, component, resource identifier, kind and operation. No owner name +// or namespace appears, so the same handful of series covers three owners or +// three thousand, and no series needs removing when an owner is deleted. That +// holds only while every resource identifier stays low-cardinality, which is +// the contract WithMetricsIdentifier documents on the resource builders. +type Collectors struct { + applies *prometheus.CounterVec + errors *prometheus.CounterVec +} + +// NewCollectors creates the framework's resource-level collectors. Register the +// result with a Prometheus registry, typically controller-runtime's. +func NewCollectors() *Collectors { + return &Collectors{ + applies: prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "ocf_resource_apply_total", + Help: "Framework applies of a managed resource, by outcome.", + }, applyLabels), + errors: prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "ocf_resource_apply_errors_total", + Help: "Failed framework applies of a managed resource.", + }, errorLabels), + } +} + +// Describe implements prometheus.Collector. +func (c *Collectors) Describe(ch chan<- *prometheus.Desc) { + c.applies.Describe(ch) + c.errors.Describe(ch) +} + +// Collect implements prometheus.Collector. +func (c *Collectors) Collect(ch chan<- prometheus.Metric) { + c.applies.Collect(ch) + c.errors.Collect(ch) +} + +var _ prometheus.Collector = (*Collectors)(nil) + +// Recorder is the framework's Prometheus implementation of +// [component.MetricsRecorder]. Create one per controller with [NewRecorder] and +// assign it to [component.ReconcileContext.Metrics]. +// +// Both halves are independently optional: a Recorder built without a conditions +// gauge records only resource metrics, and one built without collectors records +// only condition metrics. +type Recorder struct { + controller string + conditions *ocm.ConditionMetricRecorder + collectors *Collectors +} + +var _ component.MetricsRecorder = (*Recorder)(nil) + +// NewRecorder creates a Recorder for the named controller. +// +// controller is the value of the `controller` label on every series the +// recorder emits, condition metrics included, so it must match the name used +// for that controller elsewhere. conditions and collectors are the shared, +// registered collectors; passing nil for either disables that half of the +// recording rather than panicking at reconcile time. +func NewRecorder( + controller string, conditions *ocm.OperatorConditionsGauge, collectors *Collectors, +) *Recorder { + r := &Recorder{controller: controller, collectors: collectors} + if conditions != nil { + r.conditions = &ocm.ConditionMetricRecorder{ + Controller: controller, + OperatorConditionsGauge: conditions, + } + } + return r +} + +// RecordConditionFor records a condition change for the given object and kind. +// It is a no-op when the recorder was built without a conditions gauge. +func (r *Recorder) RecordConditionFor( + kind string, object ocm.ObjectLike, + conditionType, conditionStatus, conditionReason string, lastTransitionTime time.Time, + extraLabelValues ...string, +) { + if r.conditions == nil { + return + } + r.conditions.RecordConditionFor( + kind, object, conditionType, conditionStatus, conditionReason, lastTransitionTime, + extraLabelValues..., + ) +} + +// RemoveConditionsFor deletes every condition metric for the given object, +// returning the number of time series removed. Call it when the object is +// deleted, so its condition series do not outlive it. +// +// There is deliberately no counterpart for the resource-level metrics: those +// series carry no owner identity, so they do not accumulate per object, and +// deleting a counter mid-flight reads downstream as a counter reset. +// +// It returns zero when the recorder was built without a conditions gauge. +func (r *Recorder) RemoveConditionsFor(kind string, object ocm.ObjectLike) int { + if r.conditions == nil { + return 0 + } + return r.conditions.RemoveConditionsFor(kind, object) +} + +// RecordResourceApply records one framework apply of a managed resource. It is +// a no-op when the recorder was built without collectors. +func (r *Recorder) RecordResourceApply( + labels component.ResourceMetricLabels, operation concepts.ConvergingOperation, +) { + if r.collectors == nil { + return + } + r.collectors.applies.WithLabelValues( + r.controller, labels.OwnerKind, labels.Component, labels.Identifier, labels.Kind, + strings.ToLower(string(operation)), + ).Inc() +} + +// RecordResourceApplyError records one failed framework apply of a managed +// resource. It is a no-op when the recorder was built without collectors. +func (r *Recorder) RecordResourceApplyError(labels component.ResourceMetricLabels) { + if r.collectors == nil { + return + } + r.collectors.errors.WithLabelValues( + r.controller, labels.OwnerKind, labels.Component, labels.Identifier, labels.Kind, + ).Inc() +} diff --git a/pkg/metrics/metrics_test.go b/pkg/metrics/metrics_test.go new file mode 100644 index 00000000..47155941 --- /dev/null +++ b/pkg/metrics/metrics_test.go @@ -0,0 +1,137 @@ +package metrics_test + +import ( + "strings" + "testing" + "time" + + "github.com/prometheus/client_golang/prometheus/testutil" + ocm "github.com/sourcehawk/go-crd-condition-metrics/pkg/crd-condition-metrics" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/sourcehawk/operator-component-framework/pkg/component" + "github.com/sourcehawk/operator-component-framework/pkg/component/concepts" + "github.com/sourcehawk/operator-component-framework/pkg/metrics" +) + +func testLabels() component.ResourceMetricLabels { + return component.ResourceMetricLabels{ + OwnerKind: "WebApp", + Component: "web", + Identifier: "tls", + Kind: "Secret", + } +} + +func TestRecorderSatisfiesMetricsRecorder(t *testing.T) { + var recorder component.MetricsRecorder = metrics.NewRecorder("webapp-controller", nil, nil) + assert.NotNil(t, recorder) +} + +func TestRecordResourceApply(t *testing.T) { + collectors := metrics.NewCollectors() + recorder := metrics.NewRecorder("webapp-controller", nil, collectors) + + recorder.RecordResourceApply(testLabels(), concepts.ConvergingOperationNone) + recorder.RecordResourceApply(testLabels(), concepts.ConvergingOperationNone) + recorder.RecordResourceApply(testLabels(), concepts.ConvergingOperationUpdated) + + expected := ` +# HELP ocf_resource_apply_total Framework applies of a managed resource, by outcome. +# TYPE ocf_resource_apply_total counter +ocf_resource_apply_total{component="web",controller="webapp-controller",kind="Secret",operation="none",owner_kind="WebApp",resource="tls"} 2 +ocf_resource_apply_total{component="web",controller="webapp-controller",kind="Secret",operation="updated",owner_kind="WebApp",resource="tls"} 1 +` + require.NoError(t, testutil.CollectAndCompare( + collectors, strings.NewReader(expected), "ocf_resource_apply_total", + )) +} + +func TestRecordResourceApplyLowercasesTheOperation(t *testing.T) { + collectors := metrics.NewCollectors() + recorder := metrics.NewRecorder("webapp-controller", nil, collectors) + + recorder.RecordResourceApply(testLabels(), concepts.ConvergingOperationCreated) + + expected := ` +# HELP ocf_resource_apply_total Framework applies of a managed resource, by outcome. +# TYPE ocf_resource_apply_total counter +ocf_resource_apply_total{component="web",controller="webapp-controller",kind="Secret",operation="created",owner_kind="WebApp",resource="tls"} 1 +` + require.NoError(t, testutil.CollectAndCompare( + collectors, strings.NewReader(expected), "ocf_resource_apply_total", + )) +} + +func TestRecordResourceApplyError(t *testing.T) { + collectors := metrics.NewCollectors() + recorder := metrics.NewRecorder("webapp-controller", nil, collectors) + + recorder.RecordResourceApplyError(testLabels()) + + expected := ` +# HELP ocf_resource_apply_errors_total Failed framework applies of a managed resource. +# TYPE ocf_resource_apply_errors_total counter +ocf_resource_apply_errors_total{component="web",controller="webapp-controller",kind="Secret",owner_kind="WebApp",resource="tls"} 1 +` + require.NoError(t, testutil.CollectAndCompare( + collectors, strings.NewReader(expected), "ocf_resource_apply_errors_total", + )) +} + +func TestSeparateControllersShareCollectors(t *testing.T) { + collectors := metrics.NewCollectors() + web := metrics.NewRecorder("webapp-controller", nil, collectors) + db := metrics.NewRecorder("db-controller", nil, collectors) + + web.RecordResourceApply(testLabels(), concepts.ConvergingOperationNone) + db.RecordResourceApply(testLabels(), concepts.ConvergingOperationNone) + + expected := ` +# HELP ocf_resource_apply_total Framework applies of a managed resource, by outcome. +# TYPE ocf_resource_apply_total counter +ocf_resource_apply_total{component="web",controller="db-controller",kind="Secret",operation="none",owner_kind="WebApp",resource="tls"} 1 +ocf_resource_apply_total{component="web",controller="webapp-controller",kind="Secret",operation="none",owner_kind="WebApp",resource="tls"} 1 +` + require.NoError(t, testutil.CollectAndCompare( + collectors, strings.NewReader(expected), "ocf_resource_apply_total", + )) +} + +func TestNilCollectorsSkipResourceMetrics(t *testing.T) { + recorder := metrics.NewRecorder("webapp-controller", nil, nil) + + assert.NotPanics(t, func() { + recorder.RecordResourceApply(testLabels(), concepts.ConvergingOperationCreated) + recorder.RecordResourceApplyError(testLabels()) + }) +} + +func TestNilConditionsGaugeSkipsConditionMetrics(t *testing.T) { + recorder := metrics.NewRecorder("webapp-controller", nil, metrics.NewCollectors()) + + assert.NotPanics(t, func() { + recorder.RecordConditionFor("WebApp", fakeObject{}, "Ready", "True", "AllGood", time.Now()) + }) + assert.Zero(t, recorder.RemoveConditionsFor("WebApp", fakeObject{})) +} + +func TestConditionMetricsAreRecordedThroughTheGauge(t *testing.T) { + gauge := ocm.NewOperatorConditionsGauge("ocf_test") + recorder := metrics.NewRecorder("webapp-controller", gauge, nil) + + recorder.RecordConditionFor("WebApp", fakeObject{}, "Ready", "True", "AllGood", time.Now()) + + assert.Equal(t, 1, testutil.CollectAndCount(gauge)) + assert.Equal(t, 1, recorder.RemoveConditionsFor("WebApp", fakeObject{})) + assert.Zero(t, testutil.CollectAndCount(gauge)) +} + +// fakeObject is the minimal ocm.ObjectLike the condition recorder needs. +type fakeObject struct{} + +func (fakeObject) GetName() string { return "app" } +func (fakeObject) GetNamespace() string { return "default" } + +var _ ocm.ObjectLike = fakeObject{} diff --git a/pkg/primitives/clusterrole/builder.go b/pkg/primitives/clusterrole/builder.go index a3eab214..90362973 100644 --- a/pkg/primitives/clusterrole/builder.go +++ b/pkg/primitives/clusterrole/builder.go @@ -81,6 +81,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the ClusterRole's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `clusterrole`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/pkg/primitives/clusterrole/resource.go b/pkg/primitives/clusterrole/resource.go index 117d9553..5f050e12 100644 --- a/pkg/primitives/clusterrole/resource.go +++ b/pkg/primitives/clusterrole/resource.go @@ -30,6 +30,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Kubernetes ClusterRole object. // // The returned object implements client.Object, making it compatible with @@ -111,3 +119,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/clusterrolebinding/builder.go b/pkg/primitives/clusterrolebinding/builder.go index 6c3d2b52..a620cab8 100644 --- a/pkg/primitives/clusterrolebinding/builder.go +++ b/pkg/primitives/clusterrolebinding/builder.go @@ -84,6 +84,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the ClusterRoleBinding's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `clusterrolebinding`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/pkg/primitives/clusterrolebinding/resource.go b/pkg/primitives/clusterrolebinding/resource.go index 2a26b5ee..6be8ed90 100644 --- a/pkg/primitives/clusterrolebinding/resource.go +++ b/pkg/primitives/clusterrolebinding/resource.go @@ -29,6 +29,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Kubernetes ClusterRoleBinding object. // // The returned object implements client.Object, making it compatible with @@ -111,3 +119,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/configmap/builder.go b/pkg/primitives/configmap/builder.go index 9c0c756b..70fb748c 100644 --- a/pkg/primitives/configmap/builder.go +++ b/pkg/primitives/configmap/builder.go @@ -80,6 +80,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the ConfigMap's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `configmap`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/pkg/primitives/configmap/builder_test.go b/pkg/primitives/configmap/builder_test.go index 83325fa4..25826381 100644 --- a/pkg/primitives/configmap/builder_test.go +++ b/pkg/primitives/configmap/builder_test.go @@ -98,6 +98,35 @@ func TestExtractIntoDeclaredExtraction(t *testing.T) { assert.Equal(t, "postgres.default.svc", v) } +func TestWithMetricsIdentifier(t *testing.T) { + t.Parallel() + newBuilder := func() *Builder { + return NewBuilder(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "web-tls", Namespace: "default"}, + }) + } + + t.Run("defaults to empty so the framework labels by kind", func(t *testing.T) { + t.Parallel() + res, err := newBuilder().Build() + require.NoError(t, err) + assert.Empty(t, res.MetricsIdentifier()) + }) + + t.Run("returns the configured identifier", func(t *testing.T) { + t.Parallel() + res, err := newBuilder().WithMetricsIdentifier("tls").Build() + require.NoError(t, err) + assert.Equal(t, "tls", res.MetricsIdentifier()) + }) + + t.Run("rejects a blank identifier", func(t *testing.T) { + t.Parallel() + _, err := newBuilder().WithMetricsIdentifier(" ").Build() + assert.EqualError(t, err, "metrics identifier cannot be blank") + }) +} + func TestWithDataGuardAndOptionalDataDeclarations(t *testing.T) { t.Parallel() guarded := concepts.NewData[string]("db-host") diff --git a/pkg/primitives/configmap/resource.go b/pkg/primitives/configmap/resource.go index 1e63f0d0..309d073d 100644 --- a/pkg/primitives/configmap/resource.go +++ b/pkg/primitives/configmap/resource.go @@ -28,6 +28,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Kubernetes ConfigMap object. // // The returned object implements client.Object, making it compatible with @@ -109,3 +117,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/cronjob/builder.go b/pkg/primitives/cronjob/builder.go index e7d05c88..0b5717c9 100644 --- a/pkg/primitives/cronjob/builder.go +++ b/pkg/primitives/cronjob/builder.go @@ -140,6 +140,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the CronJob's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `cronjob`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It ensures that: diff --git a/pkg/primitives/cronjob/resource.go b/pkg/primitives/cronjob/resource.go index 889a13b7..551185ec 100644 --- a/pkg/primitives/cronjob/resource.go +++ b/pkg/primitives/cronjob/resource.go @@ -29,6 +29,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a copy of the underlying Kubernetes CronJob object. func (r *Resource) Object() (client.Object, error) { return r.base.Object() @@ -146,3 +154,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/daemonset/builder.go b/pkg/primitives/daemonset/builder.go index f6248534..9168ef97 100644 --- a/pkg/primitives/daemonset/builder.go +++ b/pkg/primitives/daemonset/builder.go @@ -172,6 +172,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the DaemonSet's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `daemonset`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It ensures that: diff --git a/pkg/primitives/daemonset/resource.go b/pkg/primitives/daemonset/resource.go index 90f199bb..1f7587d4 100644 --- a/pkg/primitives/daemonset/resource.go +++ b/pkg/primitives/daemonset/resource.go @@ -34,6 +34,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a copy of the underlying Kubernetes DaemonSet object. // // The returned object implements the client.Object interface, making it @@ -177,3 +185,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/deployment/builder.go b/pkg/primitives/deployment/builder.go index c00789b5..87182af9 100644 --- a/pkg/primitives/deployment/builder.go +++ b/pkg/primitives/deployment/builder.go @@ -179,6 +179,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the Deployment's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `deployment`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It ensures that: diff --git a/pkg/primitives/deployment/metrics_identifier_test.go b/pkg/primitives/deployment/metrics_identifier_test.go new file mode 100644 index 00000000..2cfb0928 --- /dev/null +++ b/pkg/primitives/deployment/metrics_identifier_test.go @@ -0,0 +1,41 @@ +package deployment + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// TestWithMetricsIdentifier covers the workload builder's exposure of the +// metrics identifier, the shape shared by every workload primitive. +func TestWithMetricsIdentifier(t *testing.T) { + t.Parallel() + newBuilder := func() *Builder { + return NewBuilder(&appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "web", Namespace: "default"}, + }) + } + + t.Run("defaults to empty so the framework labels by kind", func(t *testing.T) { + t.Parallel() + res, err := newBuilder().Build() + require.NoError(t, err) + assert.Empty(t, res.MetricsIdentifier()) + }) + + t.Run("returns the configured identifier", func(t *testing.T) { + t.Parallel() + res, err := newBuilder().WithMetricsIdentifier("frontend").Build() + require.NoError(t, err) + assert.Equal(t, "frontend", res.MetricsIdentifier()) + }) + + t.Run("rejects a blank identifier", func(t *testing.T) { + t.Parallel() + _, err := newBuilder().WithMetricsIdentifier(" ").Build() + assert.EqualError(t, err, "metrics identifier cannot be blank") + }) +} diff --git a/pkg/primitives/deployment/resource.go b/pkg/primitives/deployment/resource.go index 6c94329c..9823675f 100644 --- a/pkg/primitives/deployment/resource.go +++ b/pkg/primitives/deployment/resource.go @@ -34,6 +34,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a copy of the underlying Kubernetes Deployment object. // // The returned object implements the client.Object interface, making it @@ -193,3 +201,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/hpa/builder.go b/pkg/primitives/hpa/builder.go index e5242405..2e4f1f6d 100644 --- a/pkg/primitives/hpa/builder.go +++ b/pkg/primitives/hpa/builder.go @@ -151,6 +151,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the HorizontalPodAutoscaler's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `horizontalpodautoscaler`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/pkg/primitives/hpa/resource.go b/pkg/primitives/hpa/resource.go index 1d74d365..2878133f 100644 --- a/pkg/primitives/hpa/resource.go +++ b/pkg/primitives/hpa/resource.go @@ -29,6 +29,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Kubernetes HorizontalPodAutoscaler object. // // The returned object implements client.Object, making it compatible with @@ -150,3 +158,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/ingress/builder.go b/pkg/primitives/ingress/builder.go index 8e71f5d0..a45c3b14 100644 --- a/pkg/primitives/ingress/builder.go +++ b/pkg/primitives/ingress/builder.go @@ -158,6 +158,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the Ingress's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `ingress`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/pkg/primitives/ingress/resource.go b/pkg/primitives/ingress/resource.go index 7a7a16bb..647bde3b 100644 --- a/pkg/primitives/ingress/resource.go +++ b/pkg/primitives/ingress/resource.go @@ -34,6 +34,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Kubernetes Ingress object. // // The returned object implements client.Object, making it compatible with @@ -164,3 +172,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/job/builder.go b/pkg/primitives/job/builder.go index 02b94a03..ab8c192f 100644 --- a/pkg/primitives/job/builder.go +++ b/pkg/primitives/job/builder.go @@ -157,6 +157,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the Job's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `job`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It ensures that: diff --git a/pkg/primitives/job/metrics_identifier_test.go b/pkg/primitives/job/metrics_identifier_test.go new file mode 100644 index 00000000..727ff8ae --- /dev/null +++ b/pkg/primitives/job/metrics_identifier_test.go @@ -0,0 +1,41 @@ +package job + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + batchv1 "k8s.io/api/batch/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// TestWithMetricsIdentifier covers the task builder's exposure of the metrics +// identifier, the shape shared by every task primitive. +func TestWithMetricsIdentifier(t *testing.T) { + t.Parallel() + newBuilder := func() *Builder { + return NewBuilder(&batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{Name: "migrate", Namespace: "default"}, + }) + } + + t.Run("defaults to empty so the framework labels by kind", func(t *testing.T) { + t.Parallel() + res, err := newBuilder().Build() + require.NoError(t, err) + assert.Empty(t, res.MetricsIdentifier()) + }) + + t.Run("returns the configured identifier", func(t *testing.T) { + t.Parallel() + res, err := newBuilder().WithMetricsIdentifier("schema-migration").Build() + require.NoError(t, err) + assert.Equal(t, "schema-migration", res.MetricsIdentifier()) + }) + + t.Run("rejects a blank identifier", func(t *testing.T) { + t.Parallel() + _, err := newBuilder().WithMetricsIdentifier(" ").Build() + assert.EqualError(t, err, "metrics identifier cannot be blank") + }) +} diff --git a/pkg/primitives/job/resource.go b/pkg/primitives/job/resource.go index 9f09e513..623560f2 100644 --- a/pkg/primitives/job/resource.go +++ b/pkg/primitives/job/resource.go @@ -34,6 +34,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a copy of the underlying Kubernetes Job object. // // The returned object implements the client.Object interface, making it @@ -177,3 +185,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/networkpolicy/builder.go b/pkg/primitives/networkpolicy/builder.go index a82d11e6..73632590 100644 --- a/pkg/primitives/networkpolicy/builder.go +++ b/pkg/primitives/networkpolicy/builder.go @@ -81,6 +81,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the NetworkPolicy's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `networkpolicy`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/pkg/primitives/networkpolicy/resource.go b/pkg/primitives/networkpolicy/resource.go index 8c5d49e7..0f6e5524 100644 --- a/pkg/primitives/networkpolicy/resource.go +++ b/pkg/primitives/networkpolicy/resource.go @@ -29,6 +29,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Kubernetes NetworkPolicy object. // // The returned object implements client.Object, making it compatible with @@ -111,3 +119,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/pdb/builder.go b/pkg/primitives/pdb/builder.go index 99352c77..03737ac6 100644 --- a/pkg/primitives/pdb/builder.go +++ b/pkg/primitives/pdb/builder.go @@ -80,6 +80,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the PodDisruptionBudget's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `poddisruptionbudget`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/pkg/primitives/pdb/resource.go b/pkg/primitives/pdb/resource.go index f440223d..dac4ec44 100644 --- a/pkg/primitives/pdb/resource.go +++ b/pkg/primitives/pdb/resource.go @@ -30,6 +30,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Kubernetes PodDisruptionBudget object. // // The returned object implements client.Object, making it compatible with @@ -111,3 +119,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/pod/builder.go b/pkg/primitives/pod/builder.go index ad10db56..f6d7212f 100644 --- a/pkg/primitives/pod/builder.go +++ b/pkg/primitives/pod/builder.go @@ -171,6 +171,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the Pod's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `pod`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It ensures that: diff --git a/pkg/primitives/pod/resource.go b/pkg/primitives/pod/resource.go index 349ee30d..7568ab99 100644 --- a/pkg/primitives/pod/resource.go +++ b/pkg/primitives/pod/resource.go @@ -34,6 +34,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a copy of the underlying Kubernetes Pod object. // // The returned object implements the client.Object interface, making it @@ -177,3 +185,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/pv/builder.go b/pkg/primitives/pv/builder.go index 951a5ec3..639547c9 100644 --- a/pkg/primitives/pv/builder.go +++ b/pkg/primitives/pv/builder.go @@ -114,6 +114,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the PersistentVolume's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `persistentvolume`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/pkg/primitives/pv/resource.go b/pkg/primitives/pv/resource.go index 5877ecbe..074e1b59 100644 --- a/pkg/primitives/pv/resource.go +++ b/pkg/primitives/pv/resource.go @@ -29,6 +29,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Kubernetes PersistentVolume object. // // The returned object implements client.Object, making it compatible with @@ -128,3 +136,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/pvc/builder.go b/pkg/primitives/pvc/builder.go index 5fbe0a7b..55f7d01c 100644 --- a/pkg/primitives/pvc/builder.go +++ b/pkg/primitives/pvc/builder.go @@ -152,6 +152,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the PersistentVolumeClaim's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `persistentvolumeclaim`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/pkg/primitives/pvc/resource.go b/pkg/primitives/pvc/resource.go index 07d85529..45c3b321 100644 --- a/pkg/primitives/pvc/resource.go +++ b/pkg/primitives/pvc/resource.go @@ -31,6 +31,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Kubernetes PersistentVolumeClaim object. // // The returned object implements client.Object, making it compatible with @@ -158,3 +166,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/replicaset/builder.go b/pkg/primitives/replicaset/builder.go index 7a62183a..15f34dc5 100644 --- a/pkg/primitives/replicaset/builder.go +++ b/pkg/primitives/replicaset/builder.go @@ -172,6 +172,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the ReplicaSet's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `replicaset`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It ensures that: diff --git a/pkg/primitives/replicaset/resource.go b/pkg/primitives/replicaset/resource.go index 51c0fa77..01f3d360 100644 --- a/pkg/primitives/replicaset/resource.go +++ b/pkg/primitives/replicaset/resource.go @@ -34,6 +34,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a copy of the underlying Kubernetes ReplicaSet object. // // The returned object implements the client.Object interface, making it @@ -171,3 +179,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/role/builder.go b/pkg/primitives/role/builder.go index 4192ac78..e7c5deb6 100644 --- a/pkg/primitives/role/builder.go +++ b/pkg/primitives/role/builder.go @@ -80,6 +80,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the Role's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `role`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/pkg/primitives/role/resource.go b/pkg/primitives/role/resource.go index c82a9eeb..4081c35b 100644 --- a/pkg/primitives/role/resource.go +++ b/pkg/primitives/role/resource.go @@ -29,6 +29,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Kubernetes Role object. // // The returned object implements client.Object, making it compatible with @@ -110,3 +118,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/rolebinding/builder.go b/pkg/primitives/rolebinding/builder.go index 1aeaacfe..176bb070 100644 --- a/pkg/primitives/rolebinding/builder.go +++ b/pkg/primitives/rolebinding/builder.go @@ -83,6 +83,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the RoleBinding's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `rolebinding`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/pkg/primitives/rolebinding/resource.go b/pkg/primitives/rolebinding/resource.go index 6cf2a7ce..1433a1f4 100644 --- a/pkg/primitives/rolebinding/resource.go +++ b/pkg/primitives/rolebinding/resource.go @@ -28,6 +28,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Kubernetes RoleBinding object. // // The returned object implements client.Object, making it compatible with @@ -106,3 +114,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/secret/builder.go b/pkg/primitives/secret/builder.go index e070e947..8fbeb4c9 100644 --- a/pkg/primitives/secret/builder.go +++ b/pkg/primitives/secret/builder.go @@ -80,6 +80,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the Secret's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `secret`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/pkg/primitives/secret/resource.go b/pkg/primitives/secret/resource.go index 2b4ade05..91a3b309 100644 --- a/pkg/primitives/secret/resource.go +++ b/pkg/primitives/secret/resource.go @@ -28,6 +28,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Kubernetes Secret object. // // The returned object implements client.Object, making it compatible with @@ -109,3 +117,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/service/builder.go b/pkg/primitives/service/builder.go index 5b779895..f1217f98 100644 --- a/pkg/primitives/service/builder.go +++ b/pkg/primitives/service/builder.go @@ -171,6 +171,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the Service's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `service`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It ensures that: diff --git a/pkg/primitives/service/resource.go b/pkg/primitives/service/resource.go index 92031251..2a166753 100644 --- a/pkg/primitives/service/resource.go +++ b/pkg/primitives/service/resource.go @@ -68,6 +68,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Kubernetes Service object. // // The returned object implements client.Object, making it compatible with @@ -191,3 +199,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/serviceaccount/builder.go b/pkg/primitives/serviceaccount/builder.go index b1e7d0e5..4e4b5647 100644 --- a/pkg/primitives/serviceaccount/builder.go +++ b/pkg/primitives/serviceaccount/builder.go @@ -80,6 +80,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the ServiceAccount's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `serviceaccount`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/pkg/primitives/serviceaccount/resource.go b/pkg/primitives/serviceaccount/resource.go index f587d7a6..1db6db01 100644 --- a/pkg/primitives/serviceaccount/resource.go +++ b/pkg/primitives/serviceaccount/resource.go @@ -28,6 +28,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying Kubernetes ServiceAccount object. // // The returned object implements client.Object, making it compatible with @@ -109,3 +117,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/statefulset/builder.go b/pkg/primitives/statefulset/builder.go index 5e14dc61..4571476e 100644 --- a/pkg/primitives/statefulset/builder.go +++ b/pkg/primitives/statefulset/builder.go @@ -150,6 +150,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the StatefulSet's identifier for +// resource-level metrics, used as the value of the `resource` label on +// ocf_resource_apply_total and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled `statefulset`. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It ensures that: diff --git a/pkg/primitives/statefulset/resource.go b/pkg/primitives/statefulset/resource.go index 3fdf0d4d..0b7e1a49 100644 --- a/pkg/primitives/statefulset/resource.go +++ b/pkg/primitives/statefulset/resource.go @@ -34,6 +34,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a copy of the underlying Kubernetes StatefulSet object. // // The returned object implements the client.Object interface, making it @@ -175,3 +183,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/unstructured/integration/builder.go b/pkg/primitives/unstructured/integration/builder.go index 634ec089..62d20b5d 100644 --- a/pkg/primitives/unstructured/integration/builder.go +++ b/pkg/primitives/unstructured/integration/builder.go @@ -126,6 +126,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the object's identifier for resource-level +// metrics, used as the value of the `resource` label on ocf_resource_apply_total +// and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled with its lowercased kind. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if the operational status handler has not been set. diff --git a/pkg/primitives/unstructured/integration/metrics_identifier_test.go b/pkg/primitives/unstructured/integration/metrics_identifier_test.go new file mode 100644 index 00000000..4e7cc4c1 --- /dev/null +++ b/pkg/primitives/unstructured/integration/metrics_identifier_test.go @@ -0,0 +1,32 @@ +package integration + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestWithMetricsIdentifier covers the integration builder's exposure of the +// metrics identifier. The unstructured builders have no fixed kind, so the +// framework's kind default is resolved from the object's GVK at apply time. +func TestWithMetricsIdentifier(t *testing.T) { + t.Run("defaults to empty so the framework labels by kind", func(t *testing.T) { + res, err := withRequiredHandlers(NewBuilder(validObject())).Build() + require.NoError(t, err) + assert.Empty(t, res.MetricsIdentifier()) + }) + + t.Run("returns the configured identifier", func(t *testing.T) { + res, err := withRequiredHandlers(NewBuilder(validObject())). + WithMetricsIdentifier("gateway").Build() + require.NoError(t, err) + assert.Equal(t, "gateway", res.MetricsIdentifier()) + }) + + t.Run("rejects a blank identifier", func(t *testing.T) { + _, err := withRequiredHandlers(NewBuilder(validObject())). + WithMetricsIdentifier(" ").Build() + assert.EqualError(t, err, "metrics identifier cannot be blank") + }) +} diff --git a/pkg/primitives/unstructured/integration/resource.go b/pkg/primitives/unstructured/integration/resource.go index 1dbc39b6..d301a83a 100644 --- a/pkg/primitives/unstructured/integration/resource.go +++ b/pkg/primitives/unstructured/integration/resource.go @@ -35,6 +35,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying unstructured Kubernetes object. func (r *Resource) Object() (client.Object, error) { return r.base.Object() @@ -135,3 +143,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/unstructured/static/builder.go b/pkg/primitives/unstructured/static/builder.go index 8fbc8531..eec7e33f 100644 --- a/pkg/primitives/unstructured/static/builder.go +++ b/pkg/primitives/unstructured/static/builder.go @@ -90,6 +90,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the object's identifier for resource-level +// metrics, used as the value of the `resource` label on ocf_resource_apply_total +// and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled with its lowercased kind. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if: diff --git a/pkg/primitives/unstructured/static/resource.go b/pkg/primitives/unstructured/static/resource.go index 34ec6d13..f0537726 100644 --- a/pkg/primitives/unstructured/static/resource.go +++ b/pkg/primitives/unstructured/static/resource.go @@ -33,6 +33,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying unstructured Kubernetes object. func (r *Resource) Object() (client.Object, error) { return r.base.Object() @@ -104,3 +112,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/unstructured/task/builder.go b/pkg/primitives/unstructured/task/builder.go index 7b7e1928..b8e0965c 100644 --- a/pkg/primitives/unstructured/task/builder.go +++ b/pkg/primitives/unstructured/task/builder.go @@ -117,6 +117,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the object's identifier for resource-level +// metrics, used as the value of the `resource` label on ocf_resource_apply_total +// and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled with its lowercased kind. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if the converging status handler has not been set. diff --git a/pkg/primitives/unstructured/task/resource.go b/pkg/primitives/unstructured/task/resource.go index bddf019c..f789c59b 100644 --- a/pkg/primitives/unstructured/task/resource.go +++ b/pkg/primitives/unstructured/task/resource.go @@ -33,6 +33,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying unstructured Kubernetes object. func (r *Resource) Object() (client.Object, error) { return r.base.Object() @@ -127,3 +135,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/pkg/primitives/unstructured/workload/builder.go b/pkg/primitives/unstructured/workload/builder.go index 63f3b6d6..444b33e3 100644 --- a/pkg/primitives/unstructured/workload/builder.go +++ b/pkg/primitives/unstructured/workload/builder.go @@ -126,6 +126,19 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the object's identifier for resource-level +// metrics, used as the value of the `resource` label on ocf_resource_apply_total +// and ocf_resource_apply_errors_total. +// +// It is a Prometheus label value, not a Kubernetes name: it must be +// low-cardinality and stable across reconciles, never derived from a per-owner +// value such as the owning custom resource's name. When unset, the resource is +// labelled with its lowercased kind. Build rejects a blank identifier. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // Build validates the configuration and returns the initialized Resource. // // It returns an error if the converging status handler has not been set. diff --git a/pkg/primitives/unstructured/workload/resource.go b/pkg/primitives/unstructured/workload/resource.go index 5e8a5877..35d6c757 100644 --- a/pkg/primitives/unstructured/workload/resource.go +++ b/pkg/primitives/unstructured/workload/resource.go @@ -35,6 +35,14 @@ func (r *Resource) Identity() string { return r.base.Identity() } +// MetricsIdentifier returns the identifier set with +// Builder.WithMetricsIdentifier, or an empty string when none was set, in which +// case the framework labels the resource with its lowercased kind. It satisfies +// concepts.MetricsIdentifiable. +func (r *Resource) MetricsIdentifier() string { + return r.base.MetricsIdentifier() +} + // Object returns a deep copy of the underlying unstructured Kubernetes object. func (r *Resource) Object() (client.Object, error) { return r.base.Object() @@ -135,3 +143,4 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) diff --git a/plugin/skills/building-components/references/component.md b/plugin/skills/building-components/references/component.md index bd7c3a0a..9d21e7ae 100644 --- a/plugin/skills/building-components/references/component.md +++ b/plugin/skills/building-components/references/component.md @@ -646,7 +646,7 @@ recCtx := component.ReconcileContext{ Client: r.Client, // sigs.k8s.io/controller-runtime/pkg/client Scheme: r.Scheme, // *runtime.Scheme EventRecorder: r.EventRecorder, // events.EventRecorder, from manager.GetEventRecorder(name) - Metrics: r.Metrics, // component.MetricsRecorder (condition metrics), optional + Metrics: r.Metrics, // component.MetricsRecorder, optional; see Metrics APIReader: r.APIReader, // client.Reader, from manager.GetAPIReader(); direct reads on a status conflict Owner: owner, // the CRD that owns this component } @@ -655,9 +655,9 @@ err = comp.Reconcile(ctx, recCtx) ``` Dependencies are passed explicitly so components stay testable and decoupled from global state. The `Metrics` field is -optional; when set, the framework records Prometheus metrics for every condition reported during a reconcile, using the -recorder from [go-crd-condition-metrics](https://github.com/sourcehawk/go-crd-condition-metrics). Leave it `nil` to opt -out. +optional; when set, the framework records Prometheus metrics for every condition reported during a reconcile and for +every resource it applies. Use the recorder from `pkg/metrics`, or implement `component.MetricsRecorder` to record +elsewhere. Leave it `nil` to opt out. See [Metrics](#metrics). `EventRecorder` takes a `k8s.io/client-go/tools/events.EventRecorder`. The manager accessor that returns one, `GetEventRecorder(name)`, was added in controller-runtime v0.23; on v0.22.x, build the recorder from client-go instead, @@ -763,7 +763,7 @@ follows the server, and the next reconcile stages it again. source. After a successful update, `FlushStatus` records metrics for every condition on the owner. If `Metrics` is `nil`, -recording is skipped. +recording is skipped. See [Metrics](#metrics). This split is what lets a controller with several components stage several conditions during one reconcile and persist them in a single write. Persisting after each component would race the components' writes and produce 409 conflicts. See @@ -821,6 +821,91 @@ applies to a status-only controller too, as in the validation-only example above app.Status.ObservedGeneration = app.Generation ``` +## Metrics + +The framework records two families of Prometheus metrics through the single `ReconcileContext.Metrics` recorder: +**condition metrics**, one gauge per owner and condition type, and **resource metrics**, counters describing what the +framework did to each managed resource. + +`pkg/metrics` implements both. Build the collectors once per process and register them with controller-runtime's +registry, then give each controller its own recorder: + +```go +var ( + conditions = ocm.NewOperatorConditionsGauge("myoperator") + collectors = metrics.NewCollectors() +) + +func init() { + ctrlmetrics.Registry.MustRegister(conditions, collectors) +} + +recCtx := component.ReconcileContext{ + // ... + Metrics: metrics.NewRecorder("webapp-controller", conditions, collectors), +} +``` + +The controller name becomes the `controller` label on every series the recorder emits. Passing `nil` for either +collector disables that family; passing `nil` for `Metrics` itself disables both. + +### Resource metrics + +| Series | Type | Labels | +| --------------------------------- | ------- | ------------------------------------------------------------------------ | +| `ocf_resource_apply_total` | counter | `controller`, `owner_kind`, `component`, `resource`, `kind`, `operation` | +| `ocf_resource_apply_errors_total` | counter | `controller`, `owner_kind`, `component`, `resource`, `kind` | + +`operation` is `created`, `updated` or `none`, the same classification the framework reports through +[`ConvergingOperation`](#status-model) and the apply event. + +Both counters cover managed resources on the reconcile path and the suspension path. Read-only resources, deletions and +orphans are not applies and record nothing. + +Exactly one of the two counters moves per apply attempt, never both. The error counter covers every failure of the +attempt: mutating the desired object, the patch itself, and the classification that follows it. The one exception is a +failure so early that the framework has not worked out what it is applying, where no kind exists to label a series with; +those surface as an error condition on the owner instead. + +The reading that matters most is the `updated` rate. In steady state, a converged resource applies as `none` on every +pass and `updated` stays flat: + +```promql +rate(ocf_resource_apply_total{operation="updated"}[5m]) +``` + +A resource whose `updated` rate never settles to zero is being rewritten on every reconcile even though nothing changed. +Events report the same thing, but client-go's spam filter truncates them within seconds under exactly those conditions, +which is why the counter exists. + +### The resource identifier + +The `resource` label comes from `WithMetricsIdentifier` on the resource's builder: + +```go +secret.NewBuilder(tlsSecret). + WithMetricsIdentifier("tls"). + Build() +``` + +It is a Prometheus label value, not a name of anything in Kubernetes. When unset, the framework labels the resource with +its lowercased kind, so metrics work without any configuration. Set an identifier to tell two resources of the same kind +apart within one component, or when the object's name carries a generated suffix. `Build` rejects a blank identifier; +omit the call to accept the default. + +**The identifier must be low-cardinality and stable across reconciles**: a constant, or a value drawn from a small fixed +set. Deriving it from a per-owner value such as the owning custom resource's name creates one time series per owner, and +the framework never removes a series once created. + +Kept to that rule, the series set is bounded by the operator's static topology. No owner name or namespace appears in +these labels, so the same handful of series covers three owners or three thousand, and deleting a resource or an owner +leaves nothing behind that needs reaping. This is why there is no resource-metric counterpart to `RemoveConditionsFor`: +a resource that goes away simply stops incrementing, and deleting a counter series mid-flight would read downstream as a +counter reset and corrupt `rate()` and `increase()`. + +Per-owner detail lives in the condition metrics instead, which are keyed by owner name and namespace and are cleaned up +with `RemoveConditionsFor` when the owner is deleted. + ## Declared Data Resources inside one component pass observed values to each other through **data cells**. A cell is created in the diff --git a/plugin/skills/custom-resource-wrappers/references/custom-resource.md b/plugin/skills/custom-resource-wrappers/references/custom-resource.md index e9f7dadf..8e1a6d8f 100644 --- a/plugin/skills/custom-resource-wrappers/references/custom-resource.md +++ b/plugin/skills/custom-resource-wrappers/references/custom-resource.md @@ -715,6 +715,14 @@ func (b *Builder) WithOptionalData(cells ...concepts.DataCell) *Builder { return b } +// WithMetricsIdentifier sets the resource's identifier for resource-level metrics, +// used as the value of the `resource` label. It must be low-cardinality and stable +// across reconciles; when unset the framework labels the resource by kind. +func (b *Builder) WithMetricsIdentifier(identifier string) *Builder { + b.base.WithMetricsIdentifier(identifier) + return b +} + // WithCustomConvergeStatus overrides the default convergence status handler. func (b *Builder) WithCustomConvergeStatus( handler func(concepts.ConvergingOperation, *examplev1.MessageQueue) (concepts.AliveStatusWithReason, error), @@ -890,6 +898,7 @@ func (r *Resource) FiringSet() ([]string, error) { var _ concepts.MutationInspector = (*Resource)(nil) var _ concepts.DataProducer = (*Resource)(nil) var _ concepts.DataConsumer = (*Resource)(nil) +var _ concepts.MetricsIdentifiable = (*Resource)(nil) ``` !!! warning "Do not omit `Preview`" @@ -907,6 +916,11 @@ always if your builder exposes `ExtractInto`, `WithDataGuard`, or `WithOptionalD [build-time topology validation](component.md#build-time-validation) silently passes, `DataTopology()` omits the resource, and its cells are never cleared at the start of a reconcile. +Forward `MetricsIdentifier` whenever your builder exposes `WithMetricsIdentifier`. It satisfies +`concepts.MetricsIdentifiable`, which is how the framework reads the identifier at apply time. Without it the builder +accepts an identifier and the framework silently labels the resource by kind instead. See +[Metrics](component.md#metrics). + Forward `RecordObservation` whenever the resource may be registered read-only and declares an extraction. The framework feeds the fetched cluster object back to the resource before extraction runs; without it, the extraction would see the inert base passed to the builder rather than live cluster state. @@ -1162,10 +1176,11 @@ implications. ### Static resources Static resources have the simplest implementation. They do not participate in convergence, grace, or suspension -reporting. The builder uses `generic.NewStaticBuilder`, which supports `WithMutation`, `WithGuard`, `WithDataGuard`, and -`WithOptionalData`, plus a package-level `ExtractInto`. The resource wrapper needs only `Identity`, `Object`, `Mutate`, -`GuardStatus`, `ExtractData`, `ProducedData`, `ConsumedData`, `RecordObservation`, `Preview`, `RegisteredMutations`, and -`FiringSet`. `pkg/primitives/configmap` is a complete reference. +reporting. The builder uses `generic.NewStaticBuilder`, which supports `WithMutation`, `WithGuard`, `WithDataGuard`, +`WithOptionalData`, and `WithMetricsIdentifier`, plus a package-level `ExtractInto`. The resource wrapper needs only +`Identity`, `Object`, `Mutate`, `MetricsIdentifier`, `GuardStatus`, `ExtractData`, `ProducedData`, `ConsumedData`, +`RecordObservation`, `Preview`, `RegisteredMutations`, and `FiringSet`. `pkg/primitives/configmap` is a complete +reference. ### Task resources diff --git a/plugin/skills/using-primitives/references/primitives.md b/plugin/skills/using-primitives/references/primitives.md index ba356909..971bac1f 100644 --- a/plugin/skills/using-primitives/references/primitives.md +++ b/plugin/skills/using-primitives/references/primitives.md @@ -357,6 +357,26 @@ The interface deliberately omits operations that are not common to all three kin replica field), and the StatefulSet-only VolumeClaimTemplate methods. Reach for the concrete mutator type when you need those. +## Metrics Identifier + +Every primitive builder exposes `WithMetricsIdentifier`, which sets the value of the `resource` label on the framework's +per-resource apply counters: + +```go +res, err := secret.NewBuilder(tlsSecret). + WithMetricsIdentifier("tls"). + Build() +``` + +When unset, the framework labels the resource with its lowercased kind (`secret`, `deployment`), so the counters work +without configuration. Set an identifier to tell two resources of the same kind apart within one component, or when the +object's name carries a generated suffix. + +The identifier is a Prometheus label value, not a Kubernetes name, so it must be low-cardinality and stable across +reconciles. Never derive it from a per-owner value such as the owning custom resource's name. `Build` rejects a blank +identifier; omit the call to accept the default. See [Metrics](component.md#metrics) for the series, their labels, and +the cardinality contract. + ## Built-in Primitives | Primitive | Category | Documentation |