From 28d44be94c7d1c9abfa6b8dbdbf1148aa4ec70ed Mon Sep 17 00:00:00 2001 From: Yauheni Kaliuta Date: Tue, 1 Oct 2024 21:02:50 +0300 Subject: [PATCH] components, logger: use contextual logging approach Switch ReconcileComponent from passing logger explicitly to wrapping it into context[1][2] Makes one parameter less to pass and will allow called utilities to report component context where they are called from. No user or logging format impact until utilities takes contextual logging in use. [1] https://www.kubernetes.dev/blog/2022/05/25/contextual-logging/ [2] https://github.com/kubernetes/enhancements/blob/master/keps/sig-instrumentation/3077-contextual-logging/README.md Credits-To: Bartosz Majsak bartosz.majsak@gmail.com Signed-off-by: Yauheni Kaliuta --- components/codeflare/codeflare.go | 3 +-- components/component.go | 2 +- components/dashboard/dashboard.go | 2 +- components/datasciencepipelines/datasciencepipelines.go | 3 +-- components/kserve/kserve.go | 4 ++-- components/kueue/kueue.go | 4 ++-- components/modelmeshserving/modelmeshserving.go | 3 +-- components/modelregistry/modelregistry.go | 4 ++-- components/ray/ray.go | 4 ++-- components/trainingoperator/trainingoperator.go | 4 ++-- components/trustyai/trustyai.go | 4 ++-- components/workbenches/workbenches.go | 4 ++-- .../datasciencecluster/datasciencecluster_controller.go | 4 +++- 13 files changed, 22 insertions(+), 23 deletions(-) diff --git a/components/codeflare/codeflare.go b/components/codeflare/codeflare.go index 5e731c28ba4..c3c2ec48474 100644 --- 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,11 +73,11 @@ func (c *CodeFlare) GetComponentName() string { func (c *CodeFlare) ReconcileComponent(ctx context.Context, cli client.Client, - l logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { + l := logf.FromContext(ctx) enabled := c.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/component.go b/components/component.go index c43cef7ac92..8256fbb0542 100644 --- a/components/component.go +++ b/components/component.go @@ -82,7 +82,7 @@ type ManifestsConfig struct { type ComponentInterface interface { Init(ctx context.Context, platform cluster.Platform) error - ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger, + 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 diff --git a/components/dashboard/dashboard.go b/components/dashboard/dashboard.go index d2ed096b3bd..f8b077d4416 100644 --- a/components/dashboard/dashboard.go +++ b/components/dashboard/dashboard.go @@ -84,13 +84,13 @@ func (d *Dashboard) GetComponentName() string { func (d *Dashboard) ReconcileComponent(ctx context.Context, cli client.Client, - l logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, currentComponentExist bool, ) error { entryPath := DefaultPath + l := logf.FromContext(ctx) enabled := d.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/datasciencepipelines/datasciencepipelines.go b/components/datasciencepipelines/datasciencepipelines.go index f0066a6c544..ace22681f6b 100644 --- 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,12 +94,12 @@ func (d *DataSciencePipelines) GetComponentName() string { func (d *DataSciencePipelines) ReconcileComponent(ctx context.Context, cli client.Client, - l logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool, ) error { + l := logf.FromContext(ctx) enabled := d.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/kserve/kserve.go b/components/kserve/kserve.go index 85b739285ea..f7edf65479a 100644 --- 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,7 +111,8 @@ func (k *Kserve) GetComponentName() string { } func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client, - l logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { + owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { + l := logf.FromContext(ctx) enabled := k.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/kueue/kueue.go b/components/kueue/kueue.go index ec609317092..73147085783 100644 --- 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,8 +67,9 @@ func (k *Kueue) GetComponentName() string { return ComponentName } -func (k *Kueue) ReconcileComponent(ctx context.Context, cli client.Client, l logr.Logger, +func (k *Kueue) ReconcileComponent(ctx context.Context, cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { + l := logf.FromContext(ctx) enabled := k.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed if enabled { diff --git a/components/modelmeshserving/modelmeshserving.go b/components/modelmeshserving/modelmeshserving.go index cb1d07b7838..898d1f8617a 100644 --- 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,12 +102,12 @@ func (m *ModelMeshServing) GetComponentName() string { func (m *ModelMeshServing) ReconcileComponent(ctx context.Context, cli client.Client, - l logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool, ) error { + l := logf.FromContext(ctx) enabled := m.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/modelregistry/modelregistry.go b/components/modelregistry/modelregistry.go index dbf577ec8f8..72c9da1e381 100644 --- 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,8 +96,9 @@ func (m *ModelRegistry) GetComponentName() string { return ComponentName } -func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Client, l logr.Logger, +func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { + l := logf.FromContext(ctx) enabled := m.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/ray/ray.go b/components/ray/ray.go index c8fa30edbd4..f11ad8ec55f 100644 --- 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,8 +68,9 @@ func (r *Ray) GetComponentName() string { return ComponentName } -func (r *Ray) ReconcileComponent(ctx context.Context, cli client.Client, l logr.Logger, +func (r *Ray) ReconcileComponent(ctx context.Context, cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { + l := logf.FromContext(ctx) enabled := r.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/trainingoperator/trainingoperator.go b/components/trainingoperator/trainingoperator.go index a6a7c8f87e7..d422bc9a28a 100644 --- 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,8 +69,9 @@ func (r *TrainingOperator) GetComponentName() string { return ComponentName } -func (r *TrainingOperator) ReconcileComponent(ctx context.Context, cli client.Client, l logr.Logger, +func (r *TrainingOperator) ReconcileComponent(ctx context.Context, cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { + l := logf.FromContext(ctx) enabled := r.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/trustyai/trustyai.go b/components/trustyai/trustyai.go index 45a211e79c0..114d099901b 100644 --- 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,8 +78,9 @@ func (t *TrustyAI) GetComponentName() string { return ComponentName } -func (t *TrustyAI) ReconcileComponent(ctx context.Context, cli client.Client, l logr.Logger, +func (t *TrustyAI) ReconcileComponent(ctx context.Context, cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { + l := logf.FromContext(ctx) enabled := t.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed entryPath := DefaultPath diff --git a/components/workbenches/workbenches.go b/components/workbenches/workbenches.go index c11f1e24297..b744a1660d6 100644 --- 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,8 +110,9 @@ func (w *Workbenches) GetComponentName() string { return ComponentName } -func (w *Workbenches) ReconcileComponent(ctx context.Context, cli client.Client, l logr.Logger, +func (w *Workbenches) ReconcileComponent(ctx context.Context, cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { + l := logf.FromContext(ctx) // Set default notebooks namespace // Create rhods-notebooks namespace in managed platforms enabled := w.GetManagementState() == operatorv1.Managed diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go index 5e522e335a5..53bf173ebdc 100644 --- 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" @@ -311,7 +312,8 @@ func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context } // Reconcile component componentLogger := newComponentLogger(log, componentName, r.DataScienceCluster.DSCISpec) - err := component.ReconcileComponent(ctx, r.Client, componentLogger, instance, r.DataScienceCluster.DSCISpec, platform, installedComponentValue) + componentCtx := logf.IntoContext(ctx, componentLogger) + err := component.ReconcileComponent(componentCtx, r.Client, instance, r.DataScienceCluster.DSCISpec, platform, installedComponentValue) // TODO: replace this hack with a full refactor of component status in the future