operator: Fix structured logs common fields

operator/controllers/loki/alertingrule_controller.go

@@ -13,6 +13,7 @@ import (
 
 	lokiv1 "github.com/grafana/loki/operator/apis/loki/v1"
 	"github.com/grafana/loki/operator/controllers/loki/internal/lokistack"
+	"github.com/grafana/loki/operator/internal/external/k8s"
 )
 
 // AlertingRuleReconciler reconciles a AlertingRule object
@@ -46,8 +47,14 @@ func (r *AlertingRuleReconciler) Reconcile(ctx context.Context, _ ctrl.Request)
 
 // SetupWithManager sets up the controller with the Manager.
 func (r *AlertingRuleReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	return ctrl.NewControllerManagedBy(mgr).
+	b := ctrl.NewControllerManagedBy(mgr)
+	return r.buildController(k8s.NewCtrlBuilder(b))
+}
+
+func (r *AlertingRuleReconciler) buildController(bld k8s.Builder) error {
+	return bld.
 		For(&lokiv1.AlertingRule{}).
 		Watches(&corev1.Namespace{}, &handler.EnqueueRequestForObject{}, builder.OnlyMetadata).
+		WithLogConstructor(genericLogConstructor(r.Log, AlertingRuleCtrlName)).
 		Complete(r)
 }

operator/controllers/loki/alertingrule_controller_test.go

@@ -0,0 +1,62 @@
+package controllers
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+
+	lokiv1 "github.com/grafana/loki/operator/apis/loki/v1"
+	"github.com/grafana/loki/operator/internal/external/k8s/k8sfakes"
+)
+
+func TestAlertingRuleController_RegistersCustomResource_WithDefaultPredicates(t *testing.T) {
+	b := &k8sfakes.FakeBuilder{}
+	k := &k8sfakes.FakeClient{}
+	c := &AlertingRuleReconciler{Client: k, Log: logger, Scheme: scheme}
+
+	b.ForReturns(b)
+	b.WatchesReturns(b)
+	b.WithLogConstructorReturns(b)
+
+	err := c.buildController(b)
+	require.NoError(t, err)
+
+	require.Equal(t, 1, b.ForCallCount())
+
+	obj, _ := b.ForArgsForCall(0)
+	require.Equal(t, &lokiv1.AlertingRule{}, obj)
+}
+
+func TestAlertingRuleController_RegisterWatchesResources(t *testing.T) {
+	b := &k8sfakes.FakeBuilder{}
+	k := &k8sfakes.FakeClient{}
+	c := &AlertingRuleReconciler{Client: k, Log: logger, Scheme: scheme}
+
+	b.ForReturns(b)
+	b.WatchesReturns(b)
+	b.WithLogConstructorReturns(b)
+
+	err := c.buildController(b)
+	require.NoError(t, err)
+
+	require.Equal(t, 1, b.WatchesCallCount())
+
+	obj, _, _ := b.WatchesArgsForCall(0)
+	require.Equal(t, &corev1.Namespace{}, obj)
+}
+
+func TestAlertingRuleController_RegisterGenericLogContructor(t *testing.T) {
+	b := &k8sfakes.FakeBuilder{}
+	k := &k8sfakes.FakeClient{}
+	c := &AlertingRuleReconciler{Client: k, Log: logger, Scheme: scheme}
+
+	b.ForReturns(b)
+	b.WatchesReturns(b)
+	b.WithLogConstructorReturns(b)
+
+	err := c.buildController(b)
+	require.NoError(t, err)
+
+	require.Equal(t, 1, b.WithLogConstructorCallCount())
+}

operator/controllers/loki/certrotation_controller.go

@@ -39,12 +39,14 @@ type CertRotationReconciler struct {
 // For more details, check Reconcile and its Result here:
 // - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
 func (r *CertRotationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+	ll := logWithLokiStackRef(r.Log, req, CertRotationCtrlName)
+
 	managed, err := state.IsManaged(ctx, req, r.Client)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
 	if !managed {
-		r.Log.Info("Skipping reconciliation for unmanaged LokiStack resource", "name", req.String())
+		ll.Info("skipping reconciliation for unmanaged LokiStack resource")
 		// Stop requeueing for unmanaged LokiStack custom resources
 		return ctrl.Result{}, nil
 	}
@@ -55,27 +57,26 @@ func (r *CertRotationReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	}
 
 	checkExpiryAfter := expiryRetryAfter(rt.TargetCertRefresh)
-	r.Log.Info("Checking if LokiStack certificates expired", "name", req.String(), "interval", checkExpiryAfter.String())
+	ll.Info("checking if certificates expired", "interval", checkExpiryAfter.String())
 
 	var expired *certrotation.CertExpiredError
 
-	err = handlers.CheckCertExpiry(ctx, r.Log, req, r.Client, r.FeatureGates)
+	err = handlers.CheckCertExpiry(ctx, ll, req, r.Client, r.FeatureGates)
 	switch {
 	case errors.As(err, &expired):
-		r.Log.Info("Certificate expired", "msg", expired.Error())
+		ll.Info("certificate expired", "msg", expired.Error())
 	case err != nil:
 		return ctrl.Result{}, err
 	default:
-		r.Log.Info("Skipping cert rotation, all LokiStack certificates still valid", "name", req.String())
+		ll.Info("skipping rotation, all certificates still valid")
 		return ctrl.Result{
 			RequeueAfter: checkExpiryAfter,
 		}, nil
 	}
 
-	r.Log.Error(err, "LokiStack certificates expired", "name", req.String())
+	ll.Error(err, "certificates expired")
 	err = lokistack.AnnotateForRequiredCertRotation(ctx, r.Client, req.Name, req.Namespace)
 	if err != nil {
-		r.Log.Error(err, "failed to annotate required cert rotation", "name", req.String())
 		return ctrl.Result{}, err
 	}
 
@@ -94,6 +95,7 @@ func (r *CertRotationReconciler) buildController(bld k8s.Builder) error {
 	return bld.
 		For(&lokiv1.LokiStack{}).
 		Owns(&corev1.Secret{}).
+		WithLogConstructor(lokiStackLogConstructor(r.Log, CertRotationCtrlName)).
 		Complete(r)
 }
 

operator/controllers/loki/certrotation_controller_test.go

@@ -14,29 +14,29 @@ import (
 func TestCertRotationController_RegistersCustomResource_WithDefaultPredicates(t *testing.T) {
 	b := &k8sfakes.FakeBuilder{}
 	k := &k8sfakes.FakeClient{}
-	c := &CertRotationReconciler{Client: k, Scheme: scheme}
+	c := &CertRotationReconciler{Client: k, Log: logger, Scheme: scheme}
 
 	b.ForReturns(b)
 	b.OwnsReturns(b)
+	b.WithLogConstructorReturns(b)
 
 	err := c.buildController(b)
 	require.NoError(t, err)
 
-	// Require only one For-Call for the custom resource
 	require.Equal(t, 1, b.ForCallCount())
 
-	// Require For-call with LokiStack resource
 	obj, _ := b.ForArgsForCall(0)
 	require.Equal(t, &lokiv1.LokiStack{}, obj)
 }
 
 func TestCertRotationController_RegisterOwnedResources_WithDefaultPredicates(t *testing.T) {
 	b := &k8sfakes.FakeBuilder{}
 	k := &k8sfakes.FakeClient{}
-	c := &CertRotationReconciler{Client: k, Scheme: scheme}
+	c := &CertRotationReconciler{Client: k, Log: logger, Scheme: scheme}
 
 	b.ForReturns(b)
 	b.OwnsReturns(b)
+	b.WithLogConstructorReturns(b)
 
 	err := c.buildController(b)
 	require.NoError(t, err)
@@ -47,6 +47,21 @@ func TestCertRotationController_RegisterOwnedResources_WithDefaultPredicates(t *
 	require.Equal(t, &corev1.Secret{}, obj)
 }
 
+func TestCertRotationController_RegisterLokiStackLogContructor(t *testing.T) {
+	b := &k8sfakes.FakeBuilder{}
+	k := &k8sfakes.FakeClient{}
+	c := &CertRotationReconciler{Client: k, Log: logger, Scheme: scheme}
+
+	b.ForReturns(b)
+	b.OwnsReturns(b)
+	b.WithLogConstructorReturns(b)
+
+	err := c.buildController(b)
+	require.NoError(t, err)
+
+	require.Equal(t, 1, b.WithLogConstructorCallCount())
+}
+
 func TestCertRotationController_ExpiryRetryAfter(t *testing.T) {
 	tt := []struct {
 		desc         string

operator/controllers/loki/controllers.go

@@ -0,0 +1,53 @@
+package controllers
+
+import (
+	"github.com/go-logr/logr"
+	"k8s.io/klog/v2"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	lokiv1 "github.com/grafana/loki/operator/apis/loki/v1"
+)
+
+const (
+	AlertingRuleCtrlName        = "alertingrule"
+	CertRotationCtrlName        = "certrotation"
+	DashboardsCtrlName          = "dashboard"
+	LokiStackCtrlName           = "lokistack"
+	LokiStackZoneLabelsCtrlName = "lokistack-zone-labels"
+	RecordingRuleCtrlName       = "recordingrule"
+	RulerConfigCtrlName         = "rulerconfig"
+)
+
+type LogContructorType func(request *reconcile.Request) logr.Logger
+
+func genericLogConstructor(log logr.Logger, name string) LogContructorType {
+	return func(_ *reconcile.Request) logr.Logger {
+		log := log
+		return log.WithValues("controller", name)
+	}
+}
+
+func lokiStackLogConstructor(log logr.Logger, name string) LogContructorType {
+	return func(req *reconcile.Request) logr.Logger {
+		log := log
+		l := log.WithValues("controller", name)
+
+		if req != nil {
+			l = l.WithValues(
+				"controllerGroup", lokiv1.GroupVersion.Group,
+				"controllerKind", "LokiStack",
+				"lokistack", klog.KRef(req.Namespace, req.Name),
+			)
+		}
+
+		return l
+	}
+}
+
+func logWithLokiStackRef(log logr.Logger, req ctrl.Request, name string) logr.Logger {
+	return log.WithValues(
+		"controller", name,
+		"lokistack", klog.KRef(req.Namespace, req.Name),
+	)
+}

operator/controllers/loki/dashboards_controller.go

@@ -15,6 +15,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	lokiv1 "github.com/grafana/loki/operator/apis/loki/v1"
+	"github.com/grafana/loki/operator/internal/external/k8s"
 	"github.com/grafana/loki/operator/internal/handlers"
 )
 
@@ -37,6 +38,8 @@ type DashboardsReconciler struct {
 // Reconcile creates all LokiStack dashboard ConfigMap and PrometheusRule objects on OpenShift clusters when
 // the at least one LokiStack custom resource exists or removes all when none.
 func (r *DashboardsReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+	ll := logWithLokiStackRef(r.Log, req, DashboardsCtrlName)
+
 	var stacks lokiv1.LokiStackList
 	if err := r.List(ctx, &stacks, client.MatchingLabelsSelector{Selector: labels.Everything()}); err != nil {
 		return ctrl.Result{}, kverrors.Wrap(err, "failed to list any lokistack instances")
@@ -45,23 +48,29 @@ func (r *DashboardsReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	if len(stacks.Items) == 0 {
 		// Removes all LokiStack dashboard resources on OpenShift clusters when
 		// the last LokiStack custom resource is deleted.
-		if err := handlers.DeleteDashboards(ctx, r.Client, r.OperatorNs); err != nil {
+		if err := handlers.DeleteDashboards(ctx, ll, r.Client, r.OperatorNs); err != nil {
 			return ctrl.Result{}, kverrors.Wrap(err, "failed to delete dashboard resources")
 		}
 		return ctrl.Result{}, nil
 	}
 
 	// Creates all LokiStack dashboard resources on OpenShift clusters when
 	// the first LokiStack custom resource is created.
-	if err := handlers.CreateDashboards(ctx, r.Log, r.OperatorNs, r.Client, r.Scheme); err != nil {
-		return ctrl.Result{}, kverrors.Wrap(err, "failed to create dashboard resources", "req", req)
+	if err := handlers.CreateDashboards(ctx, ll, r.Client, r.OperatorNs); err != nil {
+		return ctrl.Result{}, kverrors.Wrap(err, "failed to create dashboard resources")
 	}
 	return ctrl.Result{}, nil
 }
 
 // SetupWithManager sets up the controller with the Manager to only call this controller on create/delete/generic events.
 func (r *DashboardsReconciler) SetupWithManager(mgr manager.Manager) error {
-	return ctrl.NewControllerManagedBy(mgr).
+	b := ctrl.NewControllerManagedBy(mgr)
+	return r.buildController(k8s.NewCtrlBuilder(b))
+}
+
+func (r *DashboardsReconciler) buildController(bld k8s.Builder) error {
+	return bld.
 		For(&lokiv1.LokiStack{}, createOrDeletesPred).
+		WithLogConstructor(lokiStackLogConstructor(r.Log, DashboardsCtrlName)).
 		Complete(r)
 }

operator/controllers/loki/dashboards_controller_test.go

@@ -0,0 +1,41 @@
+package controllers
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	lokiv1 "github.com/grafana/loki/operator/apis/loki/v1"
+	"github.com/grafana/loki/operator/internal/external/k8s/k8sfakes"
+)
+
+func TestDashboardsController_RegistersCustomResource_WithCreateOrDeletePredicates(t *testing.T) {
+	b := &k8sfakes.FakeBuilder{}
+	k := &k8sfakes.FakeClient{}
+	c := &DashboardsReconciler{Client: k, Log: logger, Scheme: scheme}
+
+	b.ForReturns(b)
+	b.WithLogConstructorReturns(b)
+
+	err := c.buildController(b)
+	require.NoError(t, err)
+
+	require.Equal(t, 1, b.ForCallCount())
+
+	obj, _ := b.ForArgsForCall(0)
+	require.Equal(t, &lokiv1.LokiStack{}, obj)
+}
+
+func TestDashboardsController_RegisterGenericLogContructor(t *testing.T) {
+	b := &k8sfakes.FakeBuilder{}
+	k := &k8sfakes.FakeClient{}
+	c := &DashboardsReconciler{Client: k, Log: logger, Scheme: scheme}
+
+	b.ForReturns(b)
+	b.WithLogConstructorReturns(b)
+
+	err := c.buildController(b)
+	require.NoError(t, err)
+
+	require.Equal(t, 1, b.WithLogConstructorCallCount())
+}

operator/controllers/loki/internal/lokistack/certrotation_discovery.go

@@ -28,13 +28,13 @@ func AnnotateForRequiredCertRotation(ctx context.Context, k k8s.Client, name, na
 			return nil
 		}
 
-		return kverrors.Wrap(err, "failed to get lokistack", "key", key)
+		return kverrors.Wrap(err, "failed to get lokistack")
 	}
 
 	ss := s.DeepCopy()
 	timeStamp := time.Now().UTC().Format(time.RFC3339)
 	if err := updateAnnotation(ctx, k, ss, certRotationRequiredAtKey, timeStamp); err != nil {
-		return kverrors.Wrap(err, fmt.Sprintf("failed to update lokistack `%s` annotation", certRotationRequiredAtKey), "key", key)
+		return kverrors.Wrap(err, fmt.Sprintf("failed to update lokistack `%s` annotation", certRotationRequiredAtKey))
 	}
 
 	return nil

operator/controllers/loki/internal/lokistack/ruler_config_discovery.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/ViaQ/logerr/v2/kverrors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	lokiv1 "github.com/grafana/loki/operator/apis/loki/v1"
@@ -28,7 +29,7 @@ func AnnotateForRulerConfig(ctx context.Context, k k8s.Client, name, namespace s
 
 	timeStamp := time.Now().UTC().Format(time.RFC3339)
 	if err := updateAnnotation(ctx, k, ss, annotationRulerConfigDiscoveredAt, timeStamp); err != nil {
-		return kverrors.Wrap(err, "failed to update lokistack `rulerConfigDiscoveredAt` annotation", "key", key)
+		return kverrors.Wrap(err, "failed to update lokistack `rulerConfigDiscoveredAt` annotation", "lokistack", klog.KRef(ss.Namespace, ss.Name))
 	}
 
 	return nil
@@ -45,7 +46,7 @@ func RemoveRulerConfigAnnotation(ctx context.Context, k k8s.Client, name, namesp
 	}
 
 	if err := removeAnnotation(ctx, k, ss, annotationRulerConfigDiscoveredAt); err != nil {
-		return kverrors.Wrap(err, "failed to update lokistack `rulerConfigDiscoveredAt` annotation", "key", key)
+		return kverrors.Wrap(err, "failed to update lokistack `rulerConfigDiscoveredAt` annotation", "lokistack", klog.KRef(ss.Namespace, ss.Name))
 	}
 
 	return nil