From 2e35273b2d03397c114e46b6c57b83ff208fbe6a Mon Sep 17 00:00:00 2001 From: Geoffrey Israel Date: Mon, 16 Oct 2023 13:21:24 +0100 Subject: [PATCH] chore(metrics-operator): improve logging (#2269) --- .../controllers/analysis/controller.go | 10 ++++---- metrics-operator/controllers/common/common.go | 11 +++++++++ .../controllers/common/common_test.go | 24 +++++++++++++++++++ .../controllers/metrics/controller.go | 18 +++++++------- 4 files changed, 51 insertions(+), 12 deletions(-) create mode 100644 metrics-operator/controllers/common/common.go create mode 100644 metrics-operator/controllers/common/common_test.go diff --git a/metrics-operator/controllers/analysis/controller.go b/metrics-operator/controllers/analysis/controller.go index a42d5549bb..54861df164 100644 --- a/metrics-operator/controllers/analysis/controller.go +++ b/metrics-operator/controllers/analysis/controller.go @@ -25,6 +25,7 @@ import ( "github.com/go-logr/logr" "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1alpha3" metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1alpha3" + ctrlcommon "github.com/keptn/lifecycle-toolkit/metrics-operator/controllers/common" common "github.com/keptn/lifecycle-toolkit/metrics-operator/controllers/common/analysis" evalType "github.com/keptn/lifecycle-toolkit/metrics-operator/controllers/common/analysis/types" "golang.org/x/exp/maps" @@ -60,17 +61,18 @@ type AnalysisReconciler struct { // For more details, check Reconcile and its AnalysisResult here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.13.0/pkg/reconcile func (a *AnalysisReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - a.Log.Info("Reconciling Analysis") + requestInfo := ctrlcommon.GetRequestInfo(req) + a.Log.Info("Reconciling Analysis", "requestInfo", requestInfo) analysis := &metricsapi.Analysis{} //retrieve analysis if err := a.Client.Get(ctx, req.NamespacedName, analysis); err != nil { if errors.IsNotFound(err) { // taking down all associated K8s resources is handled by K8s - a.Log.Info("Analysis resource not found. Ignoring since object must be deleted") + a.Log.Info("Analysis resource not found. Ignoring since object must be deleted", "requestInfo", requestInfo) return ctrl.Result{}, nil } - a.Log.Error(err, "Failed to get the Analysis") + a.Log.Error(err, "Failed to get the Analysis", "requestInfo", requestInfo) return ctrl.Result{}, err } @@ -103,7 +105,7 @@ func (a *AnalysisReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c res, err := wp.DispatchAndCollect(childCtx) if err != nil { - a.Log.Error(err, "Failed to collect all values required for the Analysis, caching collected values") + a.Log.Error(err, "Failed to collect all values required for the Analysis, caching collected values", "requestInfo", requestInfo) analysis.Status.StoredValues = res return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, a.updateStatus(ctx, analysis) } diff --git a/metrics-operator/controllers/common/common.go b/metrics-operator/controllers/common/common.go new file mode 100644 index 0000000000..fdef5d8b8f --- /dev/null +++ b/metrics-operator/controllers/common/common.go @@ -0,0 +1,11 @@ +package common + +import ctrl "sigs.k8s.io/controller-runtime" + +// GetRequestInfo extracts name and namespace from a controller request. +func GetRequestInfo(req ctrl.Request) map[string]string { + return map[string]string{ + "name": req.Name, + "namespace": req.Namespace, + } +} diff --git a/metrics-operator/controllers/common/common_test.go b/metrics-operator/controllers/common/common_test.go new file mode 100644 index 0000000000..dfb6af3bee --- /dev/null +++ b/metrics-operator/controllers/common/common_test.go @@ -0,0 +1,24 @@ +package common + +import ( + "testing" + + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" +) + +func TestGetRequestInfo(t *testing.T) { + req := ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: "example", + Namespace: "test-namespace", + }} + + info := GetRequestInfo(req) + expected := map[string]string{ + "name": "example", + "namespace": "test-namespace", + } + require.Equal(t, expected, info) +} diff --git a/metrics-operator/controllers/metrics/controller.go b/metrics-operator/controllers/metrics/controller.go index 5bbe8dd2c5..b3e1d2667a 100644 --- a/metrics-operator/controllers/metrics/controller.go +++ b/metrics-operator/controllers/metrics/controller.go @@ -25,6 +25,7 @@ import ( "github.com/go-logr/logr" metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1alpha3" + ctrlcommon "github.com/keptn/lifecycle-toolkit/metrics-operator/controllers/common" "github.com/keptn/lifecycle-toolkit/metrics-operator/controllers/common/aggregation" "github.com/keptn/lifecycle-toolkit/metrics-operator/controllers/common/providers" "k8s.io/apimachinery/pkg/api/errors" @@ -64,39 +65,40 @@ type KeptnMetricReconciler struct { // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.12.2/pkg/reconcile func (r *KeptnMetricReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - r.Log.Info("Reconciling Metric") + requestInfo := ctrlcommon.GetRequestInfo(req) + r.Log.Info("Reconciling Metric", "requestInfo", requestInfo) metric := &metricsapi.KeptnMetric{} if err := r.Client.Get(ctx, req.NamespacedName, metric); err != nil { if errors.IsNotFound(err) { // taking down all associated K8s resources is handled by K8s - r.Log.Info("Metric resource not found. Ignoring since object must be deleted") + r.Log.Info("Metric resource not found. Ignoring since object must be deleted", "requestInfo", requestInfo) return ctrl.Result{}, nil } - r.Log.Error(err, "Failed to get the Metric") + r.Log.Error(err, "Failed to get the Metric", "requestInfo", requestInfo) return ctrl.Result{}, nil } fetchTime := metric.Status.LastUpdated.Add(time.Second * time.Duration(metric.Spec.FetchIntervalSeconds)) if time.Now().Before(fetchTime) { diff := time.Until(fetchTime) - r.Log.Info("Metric has not been updated for the configured interval. Skipping") + r.Log.Info("Metric has not been updated for the configured interval. Skipping", "requestInfo", requestInfo) return ctrl.Result{Requeue: true, RequeueAfter: diff}, nil } metricProvider, err := r.fetchProvider(ctx, types.NamespacedName{Name: metric.Spec.Provider.Name, Namespace: metric.Namespace}) if err != nil { if errors.IsNotFound(err) { - r.Log.Info(err.Error() + ", ignoring error since object must be deleted") + r.Log.Info(err.Error()+", ignoring error since object must be deleted", "requestInfo", requestInfo) return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, nil } - r.Log.Error(err, "Failed to retrieve the provider") + r.Log.Error(err, "Failed to retrieve the provider", "requestInfo", requestInfo) return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } // load the provider provider, err2 := r.ProviderFactory(metricProvider.GetType(), r.Log, r.Client) if err2 != nil { - r.Log.Error(err2, "Failed to get the correct Metric Provider") + r.Log.Error(err2, "Failed to get the correct Metric Provider", "requestInfo", requestInfo) return ctrl.Result{Requeue: false}, err2 } reconcile := ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second} @@ -116,7 +118,7 @@ func (r *KeptnMetricReconciler) Reconcile(ctx context.Context, req ctrl.Request) } if err := r.Client.Status().Update(ctx, metric); err != nil { - r.Log.Error(err, "Failed to update the Metric status") + r.Log.Error(err, "Failed to update the Metric status", "requestInfo", requestInfo) return ctrl.Result{}, err }