components: wrap logger into context

Wrap the logger into Context like controller-runtime does [1][2]. Move former ConfigComponentLogger into DSC and rename it to configLoggerForComponent since it's a local function to prepare logger before it is passed to components. Components themself add their names. So, it moves the logic out of components into common place. Wrap logger into context for further passing. Ammend parameters of ReconcileComponent methods, it does not take logger anymore. [1] https://github.com/kubernetes-sigs/controller-runtime/blob/38546806f2faf5973e3321a7bd5bb3afdbb5767d/pkg/internal/controller/controller.go#L297 [2] https://github.com/kubernetes-sigs/controller-runtime/blob/38546806f2faf5973e3321a7bd5bb3afdbb5767d/pkg/internal/controller/controller.go#L111 Credits-To: Bartosz Majsak <[email protected]> Signed-off-by: Yauheni Kaliuta <[email protected]>
opendatahub-io · Sep 19, 2024 · 1182f1e · 1182f1e
1 parent 15f9b9c
commit 1182f1e
Show file tree

Hide file tree

Showing 14 changed files with 48 additions and 54 deletions.
diff --git a/components/README.md b/components/README.md
@@ -31,13 +31,12 @@ can be found [here](https://github.com/opendatahub-io/opendatahub-operator/tree/
 
     ```go
     type ComponentInterface interface {
-      ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger, owner metav1.Object, DSCISpec *dsciv1.DSCInitializationSpec, currentComponentStatus bool) error
+      ReconcileComponent(ctx context.Context, cli client.Client, owner metav1.Object, DSCISpec *dsciv1.DSCInitializationSpec, currentComponentStatus bool) error
       Cleanup(cli client.Client, DSCISpec *dsciv1.DSCInitializationSpec) error
       GetComponentName() string
       GetManagementState() operatorv1.ManagementState
       OverrideManifests(platform cluster.Platform) error
       UpdatePrometheusConfig(cli client.Client, enable bool, component string) error
-      ConfigComponentLogger(logger logr.Logger, component string, dscispec *dsciv1.DSCInitializationSpec) logr.Logger
     }
     ```
 

diff --git a/components/codeflare/codeflare.go b/components/codeflare/codeflare.go
@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"path/filepath"
 
-	"github.com/go-logr/logr"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -74,12 +73,12 @@ func (c *CodeFlare) GetComponentName() string {
 
 func (c *CodeFlare) ReconcileComponent(ctx context.Context,
 	cli client.Client,
-	logger logr.Logger,
 	owner metav1.Object,
 	dscispec *dsciv1.DSCInitializationSpec,
 	platform cluster.Platform,
 	_ bool) error {
-	l := c.ConfigComponentLogger(logger, ComponentName, dscispec)
+	l := logf.FromContext(ctx).WithName(ComponentName)
+	ctx = logf.IntoContext(ctx, l)
 
 	enabled := c.GetManagementState() == operatorv1.Managed
 	monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

diff --git a/components/component.go b/components/component.go
@@ -15,7 +15,6 @@ import (
 
 	dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
-	ctrlogger "github.com/opendatahub-io/opendatahub-operator/v2/pkg/logger"
 )
 
 // Component struct defines the basis for each OpenDataHub component configuration.
@@ -83,22 +82,17 @@ type ManifestsConfig struct {
 
 type ComponentInterface interface {
 	Init(ctx context.Context, platform cluster.Platform) error
-	ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
-		owner metav1.Object, DSCISpec *dsciv1.DSCInitializationSpec, platform cluster.Platform, currentComponentStatus bool) error
+	ReconcileComponent(ctx context.Context,
+		cli client.Client,
+		owner metav1.Object,
+		DSCISpec *dsciv1.DSCInitializationSpec,
+		platform cluster.Platform,
+		currentComponentStatus bool) error
 	Cleanup(ctx context.Context, cli client.Client, owner metav1.Object, DSCISpec *dsciv1.DSCInitializationSpec) error
 	GetComponentName() string
 	GetManagementState() operatorv1.ManagementState
 	OverrideManifests(ctx context.Context, platform cluster.Platform) error
 	UpdatePrometheusConfig(cli client.Client, logger logr.Logger, enable bool, component string) error
-	ConfigComponentLogger(logger logr.Logger, component string, dscispec *dsciv1.DSCInitializationSpec) logr.Logger
-}
-
-// extend origal ConfigLoggers to include component name.
-func (c *Component) ConfigComponentLogger(logger logr.Logger, component string, dscispec *dsciv1.DSCInitializationSpec) logr.Logger {
-	if dscispec.DevFlags != nil {
-		return ctrlogger.ConfigLoggers(dscispec.DevFlags.LogMode).WithName("DSC.Components." + component)
-	}
-	return logger.WithName("DSC.Components." + component)
 }
 
 // UpdatePrometheusConfig update prometheus-configs.yaml to include/exclude <component>.rules

diff --git a/components/dashboard/dashboard.go b/components/dashboard/dashboard.go
@@ -96,20 +96,13 @@ func (d *Dashboard) GetComponentName() string {
 
 func (d *Dashboard) ReconcileComponent(ctx context.Context,
 	cli client.Client,
-	logger logr.Logger,
 	owner metav1.Object,
 	dscispec *dsciv1.DSCInitializationSpec,
 	platform cluster.Platform,
 	currentComponentExist bool,
 ) error {
-	var l logr.Logger
-
-	if platform == cluster.SelfManagedRhods || platform == cluster.ManagedRhods {
-		l = d.ConfigComponentLogger(logger, ComponentNameDownstream, dscispec)
-	} else {
-		l = d.ConfigComponentLogger(logger, ComponentNameUpstream, dscispec)
-	}
-
+	l := logf.FromContext(ctx).WithName(componentName(platform))
+	ctx = logf.IntoContext(ctx, l)
 	entryPath := DefaultPath
 	enabled := d.GetManagementState() == operatorv1.Managed
 	monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

diff --git a/components/datasciencepipelines/datasciencepipelines.go b/components/datasciencepipelines/datasciencepipelines.go
@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"path/filepath"
 
-	"github.com/go-logr/logr"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -95,13 +94,13 @@ func (d *DataSciencePipelines) GetComponentName() string {
 
 func (d *DataSciencePipelines) ReconcileComponent(ctx context.Context,
 	cli client.Client,
-	logger logr.Logger,
 	owner metav1.Object,
 	dscispec *dsciv1.DSCInitializationSpec,
 	platform cluster.Platform,
 	_ bool,
 ) error {
-	l := d.ConfigComponentLogger(logger, ComponentName, dscispec)
+	l := logf.FromContext(ctx).WithName(ComponentName)
+	ctx = logf.IntoContext(ctx, l)
 	enabled := d.GetManagementState() == operatorv1.Managed
 	monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed
 

diff --git a/components/kserve/kserve.go b/components/kserve/kserve.go
@@ -8,7 +8,6 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/go-logr/logr"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -112,8 +111,9 @@ func (k *Kserve) GetComponentName() string {
 }
 
 func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,
-	logger logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
-	l := k.ConfigComponentLogger(logger, ComponentName, dscispec)
+	owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
+	l := logf.FromContext(ctx).WithName(ComponentName)
+	ctx = logf.IntoContext(ctx, l)
 
 	enabled := k.GetManagementState() == operatorv1.Managed
 	monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

diff --git a/components/kueue/kueue.go b/components/kueue/kueue.go
@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"path/filepath"
 
-	"github.com/go-logr/logr"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -68,9 +67,10 @@ func (k *Kueue) GetComponentName() string {
 	return ComponentName
 }
 
-func (k *Kueue) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
+func (k *Kueue) ReconcileComponent(ctx context.Context, cli client.Client,
 	owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
-	l := k.ConfigComponentLogger(logger, ComponentName, dscispec)
+	l := logf.FromContext(ctx).WithName(ComponentName)
+	ctx = logf.IntoContext(ctx, l)
 
 	enabled := k.GetManagementState() == operatorv1.Managed
 	monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

diff --git a/components/modelmeshserving/modelmeshserving.go b/components/modelmeshserving/modelmeshserving.go
@@ -8,7 +8,6 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/go-logr/logr"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -103,13 +102,13 @@ func (m *ModelMeshServing) GetComponentName() string {
 
 func (m *ModelMeshServing) ReconcileComponent(ctx context.Context,
 	cli client.Client,
-	logger logr.Logger,
 	owner metav1.Object,
 	dscispec *dsciv1.DSCInitializationSpec,
 	platform cluster.Platform,
 	_ bool,
 ) error {
-	l := m.ConfigComponentLogger(logger, ComponentName, dscispec)
+	l := logf.FromContext(ctx).WithName(ComponentName)
+	ctx = logf.IntoContext(ctx, l)
 	enabled := m.GetManagementState() == operatorv1.Managed
 	monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed
 

diff --git a/components/modelregistry/modelregistry.go b/components/modelregistry/modelregistry.go
@@ -10,7 +10,6 @@ import (
 	"strings"
 	"text/template"
 
-	"github.com/go-logr/logr"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -97,9 +96,10 @@ func (m *ModelRegistry) GetComponentName() string {
 	return ComponentName
 }
 
-func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
+func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Client,
 	owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
-	l := m.ConfigComponentLogger(logger, ComponentName, dscispec)
+	l := logf.FromContext(ctx).WithName(ComponentName)
+	ctx = logf.IntoContext(ctx, l)
 	enabled := m.GetManagementState() == operatorv1.Managed
 	monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed
 

diff --git a/components/ray/ray.go b/components/ray/ray.go
@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"path/filepath"
 
-	"github.com/go-logr/logr"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -69,9 +68,10 @@ func (r *Ray) GetComponentName() string {
 	return ComponentName
 }
 
-func (r *Ray) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
+func (r *Ray) ReconcileComponent(ctx context.Context, cli client.Client,
 	owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
-	l := r.ConfigComponentLogger(logger, ComponentName, dscispec)
+	l := logf.FromContext(ctx).WithName(ComponentName)
+	ctx = logf.IntoContext(ctx, l)
 
 	enabled := r.GetManagementState() == operatorv1.Managed
 	monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

diff --git a/components/trainingoperator/trainingoperator.go b/components/trainingoperator/trainingoperator.go
@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"path/filepath"
 
-	"github.com/go-logr/logr"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -70,9 +69,10 @@ func (r *TrainingOperator) GetComponentName() string {
 	return ComponentName
 }
 
-func (r *TrainingOperator) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
+func (r *TrainingOperator) ReconcileComponent(ctx context.Context, cli client.Client,
 	owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
-	l := r.ConfigComponentLogger(logger, ComponentName, dscispec)
+	l := logf.FromContext(ctx).WithName(ComponentName)
+	ctx = logf.IntoContext(ctx, l)
 
 	enabled := r.GetManagementState() == operatorv1.Managed
 	monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

diff --git a/components/trustyai/trustyai.go b/components/trustyai/trustyai.go
@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"path/filepath"
 
-	"github.com/go-logr/logr"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -79,9 +78,10 @@ func (t *TrustyAI) GetComponentName() string {
 	return ComponentName
 }
 
-func (t *TrustyAI) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
+func (t *TrustyAI) ReconcileComponent(ctx context.Context, cli client.Client,
 	owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
-	l := t.ConfigComponentLogger(logger, ComponentName, dscispec)
+	l := logf.FromContext(ctx).WithName(ComponentName)
+	ctx = logf.IntoContext(ctx, l)
 
 	enabled := t.GetManagementState() == operatorv1.Managed
 	monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

diff --git a/components/workbenches/workbenches.go b/components/workbenches/workbenches.go
@@ -8,7 +8,6 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/go-logr/logr"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -111,9 +110,10 @@ func (w *Workbenches) GetComponentName() string {
 	return ComponentName
 }
 
-func (w *Workbenches) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
+func (w *Workbenches) ReconcileComponent(ctx context.Context, cli client.Client,
 	owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
-	l := w.ConfigComponentLogger(logger, ComponentName, dscispec)
+	l := logf.FromContext(ctx).WithName(ComponentName)
+	ctx = logf.IntoContext(ctx, l)
 
 	// Set default notebooks namespace
 	// Create rhods-notebooks namespace in managed platforms

diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go
@@ -46,6 +46,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
@@ -56,6 +57,7 @@ import (
 	"github.com/opendatahub-io/opendatahub-operator/v2/components/modelregistry"
 	"github.com/opendatahub-io/opendatahub-operator/v2/controllers/status"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
+	ctrlogger "github.com/opendatahub-io/opendatahub-operator/v2/pkg/logger"
 	annotations "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade"
@@ -80,6 +82,13 @@ const (
 	finalizerName = "datasciencecluster.opendatahub.io/finalizer"
 )
 
+func configLoggerForComponent(logger logr.Logger, dscispec *dsciv1.DSCInitializationSpec) logr.Logger {
+	if dscispec.DevFlags != nil {
+		logger = ctrlogger.ConfigLoggers(dscispec.DevFlags.LogMode)
+	}
+	return logger.WithName("DSC.Components")
+}
+
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
 func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:maintidx,gocyclo
@@ -314,7 +323,9 @@ func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context
 		r.Log.Error(err, "Failed to determine platform")
 		return instance, err
 	}
-	err = component.ReconcileComponent(ctx, r.Client, r.Log, instance, r.DataScienceCluster.DSCISpec, platform, installedComponentValue)
+
+	ctxComponent := logf.IntoContext(ctx, configLoggerForComponent(r.Log, r.DataScienceCluster.DSCISpec))
+	err = component.ReconcileComponent(ctxComponent, r.Client, instance, r.DataScienceCluster.DSCISpec, platform, installedComponentValue)
 
 	// TODO: replace this hack with a full refactor of component status in the future