Skip to content

Commit

Permalink
chore(metrics-operator): improve logging (#2269)
Browse files Browse the repository at this point in the history
  • Loading branch information
geoffrey1330 authored Oct 16, 2023
1 parent 91dd45e commit 2e35273
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 12 deletions.
10 changes: 6 additions & 4 deletions metrics-operator/controllers/analysis/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
}
Expand Down
11 changes: 11 additions & 0 deletions metrics-operator/controllers/common/common.go
Original file line number Diff line number Diff line change
@@ -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,
}
}
24 changes: 24 additions & 0 deletions metrics-operator/controllers/common/common_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
18 changes: 10 additions & 8 deletions metrics-operator/controllers/metrics/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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}
Expand All @@ -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
}

Expand Down

0 comments on commit 2e35273

Please sign in to comment.