From 4295000f2874cc0a3a4d018e5776ddf24cddecf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Sun, 16 Aug 2026 19:56:18 +0200 Subject: [PATCH 1/9] feat(generic): add a stable metrics identifier to resource builders Resources had no low-cardinality identity. Identity() embeds the Kubernetes name, so any per-resource metric keyed by it is unbounded for generated or per-CR names, and useless for asking which resource type is misbehaving. WithMetricsIdentifier records a stable label value on the resource, read by the framework through the new concepts.MetricsIdentifiable capability. The value is a Prometheus label, not a Kubernetes name: Build rejects a blank one, and omitting the call leaves the framework free to apply its own default. Co-Authored-By: Claude Fable 5 --- pkg/component/concepts/metrics.go | 14 +++++++ pkg/generic/builder_base.go | 34 +++++++++++++++++ pkg/generic/builder_integration.go | 7 ++++ pkg/generic/builder_static.go | 7 ++++ pkg/generic/builder_task.go | 7 ++++ pkg/generic/builder_workload.go | 7 ++++ pkg/generic/metrics_identifier_test.go | 53 ++++++++++++++++++++++++++ pkg/generic/resource_base.go | 12 ++++++ 8 files changed, 141 insertions(+) create mode 100644 pkg/component/concepts/metrics.go create mode 100644 pkg/generic/metrics_identifier_test.go 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/generic/builder_base.go b/pkg/generic/builder_base.go index 3c63434e..8991ef1b 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 BaseRes.MetricsIdent 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.MetricsIdent = 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.MetricsIdent) == "" { + 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..86a56ba4 100644 --- a/pkg/generic/resource_base.go +++ b/pkg/generic/resource_base.go @@ -15,6 +15,11 @@ type BaseResource[T client.Object, M FeatureMutator] struct { IdentityFunc func(T) string + // MetricsIdent is the value the framework uses for the `resource` label on + // resource-level metrics. An empty value means the framework's default + // applies. Set it with BaseBuilder.WithMetricsIdentifier. + MetricsIdent 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 +45,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.MetricsIdent +} + // 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. From 619cc28fa648e843d53835bd65a5fed9afb1b048 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:00:18 +0200 Subject: [PATCH 2/9] feat(metrics): add resource-level recorders and a Prometheus implementation MetricsRecorder now covers resource applies as well as conditions, carrying the label set in a struct so a later label is not a signature break and four same-typed arguments cannot be transposed at a call site. The new metrics package implements it: ocf_resource_apply_total and ocf_resource_apply_errors_total, keyed only by the operator's static topology so no series accumulates per owner. Both halves are independently optional, and the condition recorder is held rather than embedded so a missing gauge skips recording instead of panicking mid-reconcile. BREAKING CHANGE: a bare ocm.ConditionMetricRecorder no longer satisfies component.MetricsRecorder. Use metrics.NewRecorder, or add the two methods to a hand-written recorder. Co-Authored-By: Claude Fable 5 --- e2e/component/suite_test.go | 10 +- e2e/primitives/suite_test.go | 12 +- examples/component-prerequisites/main.go | 12 +- examples/custom-resource/main.go | 22 ++- examples/extraction-and-guards/main.go | 12 +- examples/grace-inconsistency/main.go | 12 +- examples/mutations-and-gating/main.go | 12 +- go.mod | 3 +- pkg/component/component.go | 38 ++++- pkg/component/conditions_test.go | 16 ++ pkg/component/suite_test.go | 9 +- pkg/component/zz_setup_test.go | 57 ++++++- pkg/metrics/metrics.go | 187 +++++++++++++++++++++++ pkg/metrics/metrics_test.go | 137 +++++++++++++++++ 14 files changed, 478 insertions(+), 61 deletions(-) create mode 100644 pkg/metrics/metrics.go create mode 100644 pkg/metrics/metrics_test.go 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/main.go b/examples/custom-resource/main.go index 0655b70d..1970ff4e 100644 --- a/examples/custom-resource/main.go +++ b/examples/custom-resource/main.go @@ -11,10 +11,12 @@ import ( "fmt" "os" + "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 +59,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, } 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/pkg/component/component.go b/pkg/component/component.go index 66104b2f..9d7c0324 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,14 @@ 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. + RecordResourceApplyError(labels ResourceMetricLabels) } // ReconcileContext carries the dependencies and target object for a reconciliation loop. 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/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/metrics/metrics.go b/pkg/metrics/metrics.go new file mode 100644 index 00000000..2bc026ea --- /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; see +// [generic.BaseBuilder.WithMetricsIdentifier]. +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{} From 92a9eb92dca0e22e10436714f7fc1b82449d02d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:02:10 +0200 Subject: [PATCH 3/9] feat(component): record apply and apply-error metrics per resource applyResource now emits one apply counter per pass, carrying the same converging operation as the apply event, and one error counter on every failure. Events were the only trace of a churning or failing resource, and client-go's spam filter truncates those within seconds under exactly the conditions worth watching. The GVK is resolved at the top of the function rather than just before the patch, so the object's kind labels every path. Errors that happen before it, where the framework never worked out what to apply, emit nothing rather than an unlabelled series. Closes #181 Co-Authored-By: Claude Fable 5 --- pkg/component/create.go | 96 ++++++++++--- pkg/component/create_metrics_test.go | 198 +++++++++++++++++++++++++++ 2 files changed, 272 insertions(+), 22 deletions(-) create mode 100644 pkg/component/create_metrics_test.go diff --git a/pkg/component/create.go b/pkg/component/create.go index c0f0f94c..26dce095 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 here, before anything can fail, 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,13 @@ 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) - if err := ensureGVK(obj, rec.Scheme); err != nil { - return nil, 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 +103,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 +124,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 +140,43 @@ 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 { + if configured := identifiable.MetricsIdentifier(); 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 +209,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 +280,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..831b4ee3 --- /dev/null +++ b/pkg/component/create_metrics_test.go @@ -0,0 +1,198 @@ +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("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 } From 94c1bdef7caf9f41cf70cb06ad37f80e5ece592b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:06:36 +0200 Subject: [PATCH 4/9] feat(primitives): expose WithMetricsIdentifier on every builder Each primitive builder re-exposes the generic option in the fluent style, and each primitive resource delegates MetricsIdentifier so the framework's capability assertion finds it. Without the delegation the identifier would be accepted at build time and silently ignored at apply time. The scaffolding templates carry both, so a generated wrapper is labelled the same way a built-in primitive is. Co-Authored-By: Claude Fable 5 --- internal/scaffold/data.go | 12 +++++- internal/scaffold/templates/builder.go.tmpl | 13 ++++++ internal/scaffold/templates/resource.go.tmpl | 8 ++++ .../golden/integration/builder.go.golden | 13 ++++++ .../golden/integration/resource.go.golden | 8 ++++ .../static-cluster-scoped/builder.go.golden | 13 ++++++ .../static-cluster-scoped/resource.go.golden | 8 ++++ .../testdata/golden/static/builder.go.golden | 13 ++++++ .../testdata/golden/static/resource.go.golden | 8 ++++ .../testdata/golden/task/builder.go.golden | 13 ++++++ .../testdata/golden/task/resource.go.golden | 8 ++++ .../golden/workload/builder.go.golden | 13 ++++++ .../golden/workload/resource.go.golden | 8 ++++ pkg/primitives/clusterrole/builder.go | 13 ++++++ pkg/primitives/clusterrole/resource.go | 9 ++++ pkg/primitives/clusterrolebinding/builder.go | 13 ++++++ pkg/primitives/clusterrolebinding/resource.go | 9 ++++ pkg/primitives/configmap/builder.go | 13 ++++++ pkg/primitives/configmap/builder_test.go | 29 +++++++++++++ pkg/primitives/configmap/resource.go | 9 ++++ pkg/primitives/cronjob/builder.go | 13 ++++++ pkg/primitives/cronjob/resource.go | 9 ++++ pkg/primitives/daemonset/builder.go | 13 ++++++ pkg/primitives/daemonset/resource.go | 9 ++++ pkg/primitives/deployment/builder.go | 13 ++++++ .../deployment/metrics_identifier_test.go | 41 +++++++++++++++++++ pkg/primitives/deployment/resource.go | 9 ++++ pkg/primitives/hpa/builder.go | 13 ++++++ pkg/primitives/hpa/resource.go | 9 ++++ pkg/primitives/ingress/builder.go | 13 ++++++ pkg/primitives/ingress/resource.go | 9 ++++ pkg/primitives/job/builder.go | 13 ++++++ pkg/primitives/job/metrics_identifier_test.go | 41 +++++++++++++++++++ pkg/primitives/job/resource.go | 9 ++++ pkg/primitives/networkpolicy/builder.go | 13 ++++++ pkg/primitives/networkpolicy/resource.go | 9 ++++ pkg/primitives/pdb/builder.go | 13 ++++++ pkg/primitives/pdb/resource.go | 9 ++++ pkg/primitives/pod/builder.go | 13 ++++++ pkg/primitives/pod/resource.go | 9 ++++ pkg/primitives/pv/builder.go | 13 ++++++ pkg/primitives/pv/resource.go | 9 ++++ pkg/primitives/pvc/builder.go | 13 ++++++ pkg/primitives/pvc/resource.go | 9 ++++ pkg/primitives/replicaset/builder.go | 13 ++++++ pkg/primitives/replicaset/resource.go | 9 ++++ pkg/primitives/role/builder.go | 13 ++++++ pkg/primitives/role/resource.go | 9 ++++ pkg/primitives/rolebinding/builder.go | 13 ++++++ pkg/primitives/rolebinding/resource.go | 9 ++++ pkg/primitives/secret/builder.go | 13 ++++++ pkg/primitives/secret/resource.go | 9 ++++ pkg/primitives/service/builder.go | 13 ++++++ pkg/primitives/service/resource.go | 9 ++++ pkg/primitives/serviceaccount/builder.go | 13 ++++++ pkg/primitives/serviceaccount/resource.go | 9 ++++ pkg/primitives/statefulset/builder.go | 13 ++++++ pkg/primitives/statefulset/resource.go | 9 ++++ .../unstructured/integration/builder.go | 13 ++++++ .../integration/metrics_identifier_test.go | 32 +++++++++++++++ .../unstructured/integration/resource.go | 9 ++++ pkg/primitives/unstructured/static/builder.go | 13 ++++++ .../unstructured/static/resource.go | 9 ++++ pkg/primitives/unstructured/task/builder.go | 13 ++++++ pkg/primitives/unstructured/task/resource.go | 9 ++++ .../unstructured/workload/builder.go | 13 ++++++ .../unstructured/workload/resource.go | 9 ++++ 67 files changed, 830 insertions(+), 1 deletion(-) create mode 100644 pkg/primitives/deployment/metrics_identifier_test.go create mode 100644 pkg/primitives/job/metrics_identifier_test.go create mode 100644 pkg/primitives/unstructured/integration/metrics_identifier_test.go 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/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) From 4fd24feb6edcd16e1b07028d26dd8a18e01cd726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:07:50 +0200 Subject: [PATCH 5/9] test(component): assert apply metrics settle for a rebuilt resource Reconciles a rebuilt desired object against envtest and asserts that the none counter keeps growing while updated stays at its post-change value. This is the #179 regression expressed as a metric rather than an event count, which is the point of the metric: events are gone from a real cluster within seconds under exactly these conditions. Co-Authored-By: Claude Fable 5 --- pkg/component/component_test.go | 50 +++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) 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 From 96a97bbefc0afbdb3be39dea4d8930892577878d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:12:10 +0200 Subject: [PATCH 6/9] docs(component): document per-resource apply metrics Adds a Metrics section covering both metric families, the series and their labels, the reading that catches a churning resource, and the cardinality contract the resource identifier has to honour. The primitives, custom-resource and CLI pages pick up the builder option and the delegation it needs. The custom-resource example wires the collectors and prints the counters after its two reconciles. Its CertificateRequest is named after the owner, which makes it a worked case for why the identifier is a constant rather than the object's name. Co-Authored-By: Claude Fable 5 --- .ai/base.md | 2 + .github/copilot-instructions.md | 2 + docs/cli.md | 8 +- docs/component.md | 90 +++++++++++++++++-- docs/custom-resource.md | 23 ++++- docs/getting-started.md | 2 +- docs/primitives.md | 20 +++++ examples/custom-resource/README.md | 19 ++++ examples/custom-resource/main.go | 29 ++++++ .../custom-resource/resources/certificate.go | 6 ++ .../references/component.md | 90 +++++++++++++++++-- .../references/custom-resource.md | 23 ++++- .../using-primitives/references/primitives.md | 20 +++++ 13 files changed, 311 insertions(+), 23 deletions(-) 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..bd91fbb5 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,86 @@ 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. + +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/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 1970ff4e..517c812b 100644 --- a/examples/custom-resource/main.go +++ b/examples/custom-resource/main.go @@ -10,6 +10,7 @@ import ( "context" "fmt" "os" + "strings" "github.com/prometheus/client_golang/prometheus" ocm "github.com/sourcehawk/go-crd-condition-metrics/pkg/crd-condition-metrics" @@ -91,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.") } @@ -100,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/plugin/skills/building-components/references/component.md b/plugin/skills/building-components/references/component.md index bd7c3a0a..bd91fbb5 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,86 @@ 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. + +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 | From 8c8cd486fbaa0f45b6d09d7067fec57e0f89b6f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:14:18 +0200 Subject: [PATCH 7/9] fix(component): re-assert the GVK after mutating the desired object Hoisting ensureGVK to the top of applyResource, so the object's kind could label the metrics, removed the guarantee it used to provide. A Mutate that assigns the whole struct (*current = *desired) drops the TypeMeta, and Server-Side Apply then fails with "unstructured object has no kind". The call is restored at its original position as well. It is a no-op whenever the kind is still present, and the test covering it fails without it. Co-Authored-By: Claude Fable 5 --- pkg/component/create.go | 13 ++++++++-- pkg/component/create_test.go | 49 ++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/pkg/component/create.go b/pkg/component/create.go index 26dce095..ed894f03 100644 --- a/pkg/component/create.go +++ b/pkg/component/create.go @@ -37,8 +37,8 @@ func applyResource( } // Set GVK on the object (required for SSA — builders often omit TypeMeta). - // It runs here, before anything can fail, so the object's kind is known for - // the metric labels on every later path, success and failure alike. + // 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, @@ -84,6 +84,15 @@ func applyResource( // pointer and Patch writes back into the same object. clearServerFields(obj) + // 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, 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. 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{ From b0c64e0d2f1b1ab0169a1eb007c00e1395bce715 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:15:47 +0200 Subject: [PATCH 8/9] docs(component): state exactly what the apply error counter covers Exactly one of the two counters moves per apply attempt. Spelling that out, along with the one case where neither does, saves the reader inferring the rule from the emission points. Co-Authored-By: Claude Fable 5 --- docs/component.md | 5 +++++ pkg/component/component.go | 10 +++++++--- pkg/metrics/metrics.go | 4 ++-- .../skills/building-components/references/component.md | 5 +++++ 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/docs/component.md b/docs/component.md index bd91fbb5..9d21e7ae 100644 --- a/docs/component.md +++ b/docs/component.md @@ -862,6 +862,11 @@ collector disables that family; passing `nil` for `Metrics` itself disables both 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: diff --git a/pkg/component/component.go b/pkg/component/component.go index 9d7c0324..b7fd672d 100644 --- a/pkg/component/component.go +++ b/pkg/component/component.go @@ -75,7 +75,10 @@ type MetricsRecorder interface { // read-only resources. RecordResourceApply(labels ResourceMetricLabels, operation concepts.ConvergingOperation) // RecordResourceApplyError records one failed framework apply of a managed - // resource. + // 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) } @@ -88,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/metrics/metrics.go b/pkg/metrics/metrics.go index 2bc026ea..b316d6f4 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -58,8 +58,8 @@ var ( // 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; see -// [generic.BaseBuilder.WithMetricsIdentifier]. +// 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 diff --git a/plugin/skills/building-components/references/component.md b/plugin/skills/building-components/references/component.md index bd91fbb5..9d21e7ae 100644 --- a/plugin/skills/building-components/references/component.md +++ b/plugin/skills/building-components/references/component.md @@ -862,6 +862,11 @@ collector disables that family; passing `nil` for `Metrics` itself disables both 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: From bcbfdd542736b3da8a64172855c20e23f6503fc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Sun, 16 Aug 2026 21:13:04 +0200 Subject: [PATCH 9/9] fix(generic): unexport the metrics identifier field and reject blank ones Two review findings. The BaseResource field was exported and abbreviated, which reads badly on a public struct; nothing outside pkg/generic touches it, and WithMetricsIdentifier is the only supported way to set it, so it is now unexported. Renaming it to MetricsIdentifier was not an option: Go forbids a field and a method sharing a name, and that name belongs to the accessor. The label builder also treated any non-empty identifier as configured, so a hand-written concepts.MetricsIdentifiable returning " " reached the series as its resource label. Blank now falls back to the kind, matching what the builders reject at build time. Non-blank values are still taken verbatim, since rewriting one would silently split or merge series. Co-Authored-By: Claude Fable 5 --- pkg/component/create.go | 7 ++++++- pkg/component/create_metrics_test.go | 17 +++++++++++++++++ pkg/generic/builder_base.go | 6 +++--- pkg/generic/resource_base.go | 11 ++++++----- 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/pkg/component/create.go b/pkg/component/create.go index ed894f03..7a5899c5 100644 --- a/pkg/component/create.go +++ b/pkg/component/create.go @@ -165,7 +165,12 @@ func resourceMetricLabels( kind := obj.GetObjectKind().GroupVersionKind().Kind identifier := strings.ToLower(kind) if identifiable, ok := resource.(concepts.MetricsIdentifiable); ok { - if configured := identifiable.MetricsIdentifier(); configured != "" { + // 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 } } diff --git a/pkg/component/create_metrics_test.go b/pkg/component/create_metrics_test.go index 831b4ee3..1f7ddaee 100644 --- a/pkg/component/create_metrics_test.go +++ b/pkg/component/create_metrics_test.go @@ -124,6 +124,23 @@ func TestApplyResourceMetrics(t *testing.T) { 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} diff --git a/pkg/generic/builder_base.go b/pkg/generic/builder_base.go index 8991ef1b..2520bd30 100644 --- a/pkg/generic/builder_base.go +++ b/pkg/generic/builder_base.go @@ -30,7 +30,7 @@ type BaseBuilder[T client.Object, M FeatureMutator] struct { // 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 BaseRes.MetricsIdent empty. + // (a request for the default). Both leave the resource's identifier empty. metricsIdentifierSet bool } @@ -87,7 +87,7 @@ func (b *BaseBuilder[T, M]) WithMutation(ms ...Mutation[M]) { // 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.MetricsIdent = identifier + b.BaseRes.metricsIdentifier = identifier b.metricsIdentifierSet = true } @@ -208,7 +208,7 @@ func (b *BaseBuilder[T, M]) ValidateBase() error { // 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.MetricsIdent) == "" { + if b.metricsIdentifierSet && strings.TrimSpace(b.BaseRes.metricsIdentifier) == "" { return errors.New("metrics identifier cannot be blank") } diff --git a/pkg/generic/resource_base.go b/pkg/generic/resource_base.go index 86a56ba4..db9a733f 100644 --- a/pkg/generic/resource_base.go +++ b/pkg/generic/resource_base.go @@ -15,10 +15,11 @@ type BaseResource[T client.Object, M FeatureMutator] struct { IdentityFunc func(T) string - // MetricsIdent is the value the framework uses for the `resource` label on - // resource-level metrics. An empty value means the framework's default - // applies. Set it with BaseBuilder.WithMetricsIdentifier. - MetricsIdent 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. @@ -49,7 +50,7 @@ func (r *BaseResource[T, M]) Identity() string { // 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.MetricsIdent + return r.metricsIdentifier } // RegisteredMutations returns the deduplicated Names of every mutation registered