components: move logger creation to pkg/logger (#1264)

* logger: rename ConfigLoggers to NewLogger The function actually creates a new logger with some predefined options depending of the argument. Rename to reflect actual functionality. Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com> * components: move logger creation to pkg/logger Remove ConfigComponentLogger method from ComponentInterface. In the original code this is totally internal function, not used by external interface users. But moreover by its functionality it prepares a logger from the caller to be used in component's context (creates new one with required log levels and DSC prefix or just adds the prefix to the caller's one) so make a utility function NewNamedLogger out of it. The actual component related configuration comes from DSC reconciler, so wrap it as newComponentLogger there. This is a pretty contradictive change since adding context name idiomatically belongs to the context (component), but this allows to do it in one place. Alternative is to create just a final logger in the component (l = l.WithName(componentName)). This patch changes dashboard's log name in downstream. Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com> --------- Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
opendatahub-io · Sep 26, 2024 · d918a90 · d918a90
1 parent 5648ee9
commit d918a90
Show file tree

Hide file tree

Showing 16 changed files with 35 additions and 47 deletions.
diff --git a/components/README.md b/components/README.md
@@ -37,7 +37,6 @@ can be found [here](https://github.com/opendatahub-io/opendatahub-operator/tree/
       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
@@ -59,12 +59,11 @@ func (c *CodeFlare) GetComponentName() string {
 
 func (c *CodeFlare) ReconcileComponent(ctx context.Context,
 	cli client.Client,
-	logger logr.Logger,
+	l logr.Logger,
 	owner metav1.Object,
 	dscispec *dsciv1.DSCInitializationSpec,
 	platform cluster.Platform,
 	_ bool) error {
-	l := c.ConfigComponentLogger(logger, ComponentName, dscispec)
 	var imageParamMap = map[string]string{
 		"codeflare-operator-controller-image": "RELATED_IMAGE_ODH_CODEFLARE_OPERATOR_IMAGE", // no need mcad, embedded in cfo
 	}

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.
@@ -85,15 +84,6 @@ type ComponentInterface interface {
 	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
@@ -62,20 +62,12 @@ func (d *Dashboard) GetComponentName() string {
 
 func (d *Dashboard) ReconcileComponent(ctx context.Context,
 	cli client.Client,
-	logger logr.Logger,
+	l 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)
-	}
-
 	entryPath := map[cluster.Platform]string{
 		cluster.SelfManagedRhods: PathDownstream + "/onprem",
 		cluster.ManagedRhods:     PathDownstream + "/addon",

diff --git a/components/datasciencepipelines/datasciencepipelines.go b/components/datasciencepipelines/datasciencepipelines.go
@@ -65,13 +65,12 @@ func (d *DataSciencePipelines) GetComponentName() string {
 
 func (d *DataSciencePipelines) ReconcileComponent(ctx context.Context,
 	cli client.Client,
-	logger logr.Logger,
+	l logr.Logger,
 	owner metav1.Object,
 	dscispec *dsciv1.DSCInitializationSpec,
 	platform cluster.Platform,
 	_ bool,
 ) error {
-	l := d.ConfigComponentLogger(logger, ComponentName, dscispec)
 	var imageParamMap = map[string]string{
 		// v1
 		"IMAGES_APISERVER":         "RELATED_IMAGE_ODH_ML_PIPELINES_API_SERVER_IMAGE",

diff --git a/components/kserve/kserve.go b/components/kserve/kserve.go
@@ -95,9 +95,7 @@ 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)
-
+	l logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
 	// dependentParamMap for odh-model-controller to use.
 	var dependentParamMap = map[string]string{
 		"odh-model-controller": "RELATED_IMAGE_ODH_MODEL_CONTROLLER_IMAGE",

diff --git a/components/kueue/kueue.go b/components/kueue/kueue.go
@@ -53,9 +53,8 @@ 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, l logr.Logger,
 	owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
-	l := k.ConfigComponentLogger(logger, ComponentName, dscispec)
 	var imageParamMap = map[string]string{
 		"odh-kueue-controller-image": "RELATED_IMAGE_ODH_KUEUE_CONTROLLER_IMAGE", // new kueue image
 	}

diff --git a/components/modelmeshserving/modelmeshserving.go b/components/modelmeshserving/modelmeshserving.go
@@ -73,13 +73,12 @@ func (m *ModelMeshServing) GetComponentName() string {
 
 func (m *ModelMeshServing) ReconcileComponent(ctx context.Context,
 	cli client.Client,
-	logger logr.Logger,
+	l logr.Logger,
 	owner metav1.Object,
 	dscispec *dsciv1.DSCInitializationSpec,
 	platform cluster.Platform,
 	_ bool,
 ) error {
-	l := m.ConfigComponentLogger(logger, ComponentName, dscispec)
 	var imageParamMap = map[string]string{
 		"odh-mm-rest-proxy":             "RELATED_IMAGE_ODH_MM_REST_PROXY_IMAGE",
 		"odh-modelmesh-runtime-adapter": "RELATED_IMAGE_ODH_MODELMESH_RUNTIME_ADAPTER_IMAGE",

diff --git a/components/modelregistry/modelregistry.go b/components/modelregistry/modelregistry.go
@@ -80,9 +80,8 @@ 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, l logr.Logger,
 	owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
-	l := m.ConfigComponentLogger(logger, ComponentName, dscispec)
 	var imageParamMap = map[string]string{
 		"IMAGES_MODELREGISTRY_OPERATOR": "RELATED_IMAGE_ODH_MODEL_REGISTRY_OPERATOR_IMAGE",
 		"IMAGES_GRPC_SERVICE":           "RELATED_IMAGE_ODH_MLMD_GRPC_SERVER_IMAGE",

diff --git a/components/ray/ray.go b/components/ray/ray.go
@@ -55,10 +55,8 @@ 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, l logr.Logger,
 	owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
-	l := r.ConfigComponentLogger(logger, ComponentName, dscispec)
-
 	var imageParamMap = map[string]string{
 		"odh-kuberay-operator-controller-image": "RELATED_IMAGE_ODH_KUBERAY_OPERATOR_CONTROLLER_IMAGE",
 	}

diff --git a/components/trainingoperator/trainingoperator.go b/components/trainingoperator/trainingoperator.go
@@ -55,10 +55,8 @@ 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, l logr.Logger,
 	owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
-	l := r.ConfigComponentLogger(logger, ComponentName, dscispec)
-
 	var imageParamMap = map[string]string{
 		"odh-training-operator-controller-image": "RELATED_IMAGE_ODH_TRAINING_OPERATOR_IMAGE",
 	}

diff --git a/components/trustyai/trustyai.go b/components/trustyai/trustyai.go
@@ -56,7 +56,7 @@ 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, l logr.Logger,
 	owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
 	var imageParamMap = map[string]string{
 		"trustyaiServiceImage":  "RELATED_IMAGE_ODH_TRUSTYAI_SERVICE_IMAGE",
@@ -69,8 +69,6 @@ func (t *TrustyAI) ReconcileComponent(ctx context.Context, cli client.Client, lo
 		cluster.Unknown:          PathUpstream,
 	}[platform]
 
-	l := t.ConfigComponentLogger(logger, ComponentName, dscispec)
-
 	enabled := t.GetManagementState() == operatorv1.Managed
 	monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed
 

diff --git a/components/workbenches/workbenches.go b/components/workbenches/workbenches.go
@@ -90,9 +90,8 @@ 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, l logr.Logger,
 	owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
-	l := w.ConfigComponentLogger(logger, ComponentName, dscispec)
 	var imageParamMap = map[string]string{
 		"odh-notebook-controller-image":    "RELATED_IMAGE_ODH_NOTEBOOK_CONTROLLER_IMAGE",
 		"odh-kf-notebook-controller-image": "RELATED_IMAGE_ODH_KF_NOTEBOOK_CONTROLLER_IMAGE",

diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go
@@ -56,6 +56,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"
@@ -311,7 +312,8 @@ func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context
 		}
 	}
 	// Reconcile component
-	err := component.ReconcileComponent(ctx, r.Client, r.Log, instance, r.DataScienceCluster.DSCISpec, platform, installedComponentValue)
+	componentLogger := newComponentLogger(r.Log, componentName, r.DataScienceCluster.DSCISpec)
+	err := component.ReconcileComponent(ctx, r.Client, componentLogger, instance, r.DataScienceCluster.DSCISpec, platform, installedComponentValue)
 
 	// TODO: replace this hack with a full refactor of component status in the future
 
@@ -361,6 +363,15 @@ func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context
 	return instance, nil
 }
 
+// newComponentLogger is a wrapper to add DSC name and extract log mode from DSCISpec.
+func newComponentLogger(logger logr.Logger, componentName string, dscispec *dsciv1.DSCInitializationSpec) logr.Logger {
+	mode := ""
+	if dscispec.DevFlags != nil {
+		mode = dscispec.DevFlags.LogMode
+	}
+	return ctrlogger.NewNamedLogger(logger, "DSC.Components."+componentName, mode)
+}
+
 func (r *DataScienceClusterReconciler) reportError(err error, instance *dscv1.DataScienceCluster, message string) *dscv1.DataScienceCluster {
 	r.Log.Error(err, message, "instance.Name", instance.Name)
 	r.Recorder.Eventf(instance, corev1.EventTypeWarning, "DataScienceClusterReconcileError",

diff --git a/main.go b/main.go
@@ -124,7 +124,7 @@ func main() { //nolint:funlen,maintidx
 
 	flag.Parse()
 
-	ctrl.SetLogger(logger.ConfigLoggers(logmode))
+	ctrl.SetLogger(logger.NewLogger(logmode))
 
 	// root context
 	ctx := ctrl.SetupSignalHandler()

diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go
@@ -15,6 +15,16 @@ var logLevelMapping = map[string]int{
 	"prod":    2,
 }
 
+// NewNamedLogger creates a new logger for a component.
+// If the mode is set (so can be different from the default one),
+// it will create a new logger with the specified mode's options.
+func NewNamedLogger(log logr.Logger, name string, mode string) logr.Logger {
+	if mode != "" {
+		log = NewLogger(mode)
+	}
+	return log.WithName(name)
+}
+
 // in each controller, to use different log level.
 func LogWithLevel(logger logr.Logger, level string) logr.Logger {
 	level = strings.TrimSpace(level)
@@ -27,7 +37,7 @@ func LogWithLevel(logger logr.Logger, level string) logr.Logger {
 
 // in DSC component, to use different mode for logging, e.g. development, production
 // when not set mode it falls to "default" which is used by startup main.go.
-func ConfigLoggers(mode string) logr.Logger {
+func NewLogger(mode string) logr.Logger {
 	var opts zap.Options
 	switch mode {
 	case "devel", "development": //  the most logging verbosity