From 3603ab4d221f1674332e7da0213511e1d43db001 Mon Sep 17 00:00:00 2001 From: Rajat Jindal Date: Thu, 29 Feb 2024 12:13:37 +0530 Subject: [PATCH] restructure metrics implementation per review feedback Signed-off-by: Rajat Jindal --- cmd/main.go | 13 +++--- go.mod | 2 +- internal/controller/metrics.go | 42 ------------------- internal/controller/spinapp_controller.go | 13 ++++-- .../controller/spinapp_controller_test.go | 8 ++-- .../controller/spinapp_executor_metrics.go | 36 ++++++++++++++++ internal/controller/spinapp_metrics.go | 35 ++++++++++++++++ .../controller/spinappexecutor_controller.go | 9 +++- 8 files changed, 101 insertions(+), 57 deletions(-) delete mode 100644 internal/controller/metrics.go create mode 100644 internal/controller/spinapp_executor_metrics.go create mode 100644 internal/controller/spinapp_metrics.go diff --git a/cmd/main.go b/cmd/main.go index 35cc9f60..5d49378f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -32,6 +32,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/metrics" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" spinv1 "github.com/spinkube/spin-operator/api/v1" @@ -102,16 +103,18 @@ func main() { } if err = (&controller.SpinAppReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Recorder: mgr.GetEventRecorderFor("spinapp-reconciler"), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Recorder: mgr.GetEventRecorderFor("spinapp-reconciler"), + MetricsRegistry: metrics.Registry, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "SpinApp") os.Exit(1) } if err = (&controller.SpinAppExecutorReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + MetricsRegistry: metrics.Registry, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "SpinAppExecutor") os.Exit(1) diff --git a/go.mod b/go.mod index 12a981aa..ee8fa326 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ toolchain go1.22.0 require ( github.com/go-logr/logr v1.4.1 + github.com/prometheus/client_golang v1.18.0 github.com/prometheus/common v0.48.0 github.com/stretchr/testify v1.8.4 k8s.io/api v0.29.2 @@ -41,7 +42,6 @@ require ( github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/prometheus/client_golang v1.18.0 // indirect github.com/prometheus/client_model v0.5.0 // indirect github.com/prometheus/procfs v0.12.0 // indirect github.com/spf13/pflag v1.0.5 // indirect diff --git a/internal/controller/metrics.go b/internal/controller/metrics.go deleted file mode 100644 index 6bf129a7..00000000 --- a/internal/controller/metrics.go +++ /dev/null @@ -1,42 +0,0 @@ -package controller - -import ( - "github.com/prometheus/client_golang/prometheus" - "sigs.k8s.io/controller-runtime/pkg/metrics" -) - -func init() { - registerPrometheusMetrics() -} - -func registerPrometheusMetrics() { - metrics.Registry.MustRegister(spinOperatorSpinAppInfo) - metrics.Registry.MustRegister(spinOperatorSpinAppExecutorInfo) -} - -var ( - spinOperatorSpinAppInfo = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Name: "spin_operator_spinapp_info", - Help: "info about spinapp labeled by name, namespace, executor", - }, - []string{ - "name", - "namespace", - "executor", - }, - ) - - spinOperatorSpinAppExecutorInfo = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Name: "spin_operator_spinapp_executor_info", - Help: "info about spinapp executor labeled by name, namespace, create_deployment and runtime_class_name", - }, - []string{ - "name", - "namespace", - "create_deployment", - "runtime_class_name", - }, - ) -) diff --git a/internal/controller/spinapp_controller.go b/internal/controller/spinapp_controller.go index eb59982b..39481922 100644 --- a/internal/controller/spinapp_controller.go +++ b/internal/controller/spinapp_controller.go @@ -30,6 +30,7 @@ import ( "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/metrics" "github.com/prometheus/client_golang/prometheus" spinv1 "github.com/spinkube/spin-operator/api/v1" @@ -50,9 +51,11 @@ const ( // SpinAppReconciler reconciles a SpinApp object type SpinAppReconciler struct { - Client client.Client - Scheme *runtime.Scheme - Recorder record.EventRecorder + Client client.Client + Scheme *runtime.Scheme + Recorder record.EventRecorder + MetricsRegistry metrics.RegistererGatherer + metrics *spinAppMetrics } //+kubebuilder:rbac:groups=core.spinoperator.dev,resources=spinapps,verbs=get;list;watch;create;update;patch;delete @@ -63,6 +66,8 @@ type SpinAppReconciler struct { // SetupWithManager sets up the controller with the Manager. func (r *SpinAppReconciler) SetupWithManager(mgr ctrl.Manager) error { + r.setupMetrics() + return ctrl.NewControllerManagedBy(mgr). For(&spinv1.SpinApp{}). // Owns allows watching dependency resources for any changes @@ -92,7 +97,7 @@ func (r *SpinAppReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } // record spin_operator_spinapp_info metric - spinOperatorSpinAppInfo.With(prometheus.Labels{ + r.metrics.infoGauge.With(prometheus.Labels{ "name": spinApp.Name, "namespace": spinApp.Namespace, "executor": spinApp.Spec.Executor, diff --git a/internal/controller/spinapp_controller_test.go b/internal/controller/spinapp_controller_test.go index 62fedbd0..65af124b 100644 --- a/internal/controller/spinapp_controller_test.go +++ b/internal/controller/spinapp_controller_test.go @@ -22,6 +22,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/envtest" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/metrics" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" ) @@ -103,9 +104,10 @@ func setupController(t *testing.T) (*envTestState, ctrl.Manager, *SpinAppReconci require.NoError(t, err) ctrlr := &SpinAppReconciler{ - Client: envTest.k8sClient, - Scheme: envTest.scheme, - Recorder: mgr.GetEventRecorderFor("spinapp-reconciler"), + Client: envTest.k8sClient, + Scheme: envTest.scheme, + Recorder: mgr.GetEventRecorderFor("spinapp-reconciler"), + MetricsRegistry: metrics.Registry, } require.NoError(t, ctrlr.SetupWithManager(mgr)) diff --git a/internal/controller/spinapp_executor_metrics.go b/internal/controller/spinapp_executor_metrics.go new file mode 100644 index 00000000..2f705147 --- /dev/null +++ b/internal/controller/spinapp_executor_metrics.go @@ -0,0 +1,36 @@ +package controller + +import ( + "github.com/prometheus/client_golang/prometheus" + "sigs.k8s.io/controller-runtime/pkg/metrics" +) + +type spinAppExecutorMetrics struct { + infoGauge *prometheus.GaugeVec +} + +func newSpinAppExecutorMetrics() *spinAppExecutorMetrics { + return &spinAppExecutorMetrics{ + infoGauge: prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Name: "spin_operator_spinapp_executor_info", + Help: "info about spinapp executor labeled by name, namespace, create_deployment and runtime_class_name", + }, + []string{ + "name", + "namespace", + "create_deployment", + "runtime_class_name", + }, + ), + } +} + +func (m *spinAppExecutorMetrics) Register(registry metrics.RegistererGatherer) { + registry.MustRegister(m.infoGauge) +} + +func (r *SpinAppExecutorReconciler) setupMetrics() { + r.metrics = newSpinAppExecutorMetrics() + r.metrics.Register(r.MetricsRegistry) +} diff --git a/internal/controller/spinapp_metrics.go b/internal/controller/spinapp_metrics.go new file mode 100644 index 00000000..4405c297 --- /dev/null +++ b/internal/controller/spinapp_metrics.go @@ -0,0 +1,35 @@ +package controller + +import ( + "github.com/prometheus/client_golang/prometheus" + "sigs.k8s.io/controller-runtime/pkg/metrics" +) + +type spinAppMetrics struct { + infoGauge *prometheus.GaugeVec +} + +func newSpinAppMetrics() *spinAppMetrics { + return &spinAppMetrics{ + infoGauge: prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Name: "spin_operator_spinapp_info", + Help: "info about spinapp labeled by name, namespace, executor", + }, + []string{ + "name", + "namespace", + "executor", + }, + ), + } +} + +func (m *spinAppMetrics) Register(registry metrics.RegistererGatherer) { + registry.MustRegister(m.infoGauge) +} + +func (r *SpinAppReconciler) setupMetrics() { + r.metrics = newSpinAppMetrics() + r.metrics.Register(r.MetricsRegistry) +} diff --git a/internal/controller/spinappexecutor_controller.go b/internal/controller/spinappexecutor_controller.go index 39393f48..e24ae19b 100644 --- a/internal/controller/spinappexecutor_controller.go +++ b/internal/controller/spinappexecutor_controller.go @@ -25,6 +25,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/metrics" "github.com/prometheus/client_golang/prometheus" spinv1 "github.com/spinkube/spin-operator/api/v1" @@ -34,7 +35,9 @@ import ( // SpinAppExecutorReconciler reconciles a SpinAppExecutor object type SpinAppExecutorReconciler struct { client.Client - Scheme *runtime.Scheme + Scheme *runtime.Scheme + MetricsRegistry metrics.RegistererGatherer + metrics *spinAppExecutorMetrics } //+kubebuilder:rbac:groups=core.spinoperator.dev,resources=spinappexecutors,verbs=get;list;watch;create;update;patch;delete @@ -43,6 +46,8 @@ type SpinAppExecutorReconciler struct { // SetupWithManager sets up the controller with the Manager. func (r *SpinAppExecutorReconciler) SetupWithManager(mgr ctrl.Manager) error { + r.setupMetrics() + return ctrl.NewControllerManagedBy(mgr). For(&spinv1.SpinAppExecutor{}). Complete(r) @@ -84,7 +89,7 @@ func (r *SpinAppExecutorReconciler) Reconcile(ctx context.Context, req ctrl.Requ } // record spin_operator_spinapp_executor_info metric - spinOperatorSpinAppExecutorInfo.With(prometheus.Labels{ + r.metrics.infoGauge.With(prometheus.Labels{ "name": executor.Name, "namespace": executor.Namespace, "create_deployment": fmt.Sprintf("%t", executor.Spec.CreateDeployment),