[ws-manager-mk2] logging improvements (#19635)

* [ws-manager-mk2] logging improvements * Record phase transitions, rather record phase each time we reconcile * Add OWI to log instances Also, avoid logging workspace name in workspace_types.go, as it contains org and repo info. * Fix logger WithValues * Include OWI with reconciling workspace messages * Include OWI with timeout logs * Cleanup * Rely on OWI on the context Thanks, @WVerlaek!
gitpod-io · Apr 18, 2024 · 0d1d0bd · 0d1d0bd
1 parent bcdb3ef
commit 0d1d0bd
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 2 deletions.
diff --git a/components/ws-manager-mk2/controllers/status.go b/components/ws-manager-mk2/controllers/status.go
@@ -15,6 +15,7 @@ import (
 	"github.com/gitpod-io/gitpod/common-go/tracing"
 	config "github.com/gitpod-io/gitpod/ws-manager/api/config"
 	workspacev1 "github.com/gitpod-io/gitpod/ws-manager/api/crd/v1"
+	"github.com/go-logr/logr"
 	"golang.org/x/xerrors"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -41,7 +42,15 @@ const (
 func (r *WorkspaceReconciler) updateWorkspaceStatus(ctx context.Context, workspace *workspacev1.Workspace, pods *corev1.PodList, cfg *config.Configuration) (err error) {
 	span, ctx := tracing.FromContext(ctx, "updateWorkspaceStatus")
 	defer tracing.FinishSpan(span, &err)
-	log := log.FromContext(ctx)
+	log := log.FromContext(ctx).WithValues("owi", workspace.OWI())
+	ctx = logr.NewContext(ctx, log)
+
+	oldPhase := workspace.Status.Phase
+	defer func() {
+		if oldPhase != workspace.Status.Phase {
+			log.Info("workspace phase updated", "oldPhase", oldPhase, "phase", workspace.Status.Phase)
+		}
+	}()
 
 	switch len(pods.Items) {
 	case 0:

diff --git a/components/ws-manager-mk2/controllers/timeout_controller.go b/components/ws-manager-mk2/controllers/timeout_controller.go
@@ -27,6 +27,7 @@ import (
 	"github.com/gitpod-io/gitpod/ws-manager-mk2/pkg/maintenance"
 	config "github.com/gitpod-io/gitpod/ws-manager/api/config"
 	workspacev1 "github.com/gitpod-io/gitpod/ws-manager/api/crd/v1"
+	"github.com/go-logr/logr"
 )
 
 func NewTimeoutReconciler(c client.Client, recorder record.EventRecorder, cfg config.Configuration, maintenance maintenance.Maintenance) (*TimeoutReconciler, error) {
@@ -80,6 +81,8 @@ func (r *TimeoutReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
 		// backoff.
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
+	log = log.WithValues("owi", workspace.OWI())
+	ctx = logr.NewContext(ctx, log)
 
 	if workspace.IsConditionTrue(workspacev1.WorkspaceConditionTimeout) {
 		// Workspace has already been marked as timed out.

diff --git a/components/ws-manager-mk2/controllers/workspace_controller.go b/components/ws-manager-mk2/controllers/workspace_controller.go
@@ -113,6 +113,8 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		workspace.Status.Conditions = []metav1.Condition{}
 	}
 
+	log = log.WithValues("owi", workspace.OWI())
+	ctx = logr.NewContext(ctx, log)
 	log.V(2).Info("reconciling workspace", "workspace", req.NamespacedName, "phase", workspace.Status.Phase)
 
 	workspacePods, err := r.listWorkspacePods(ctx, &workspace)
@@ -437,7 +439,7 @@ func (r *WorkspaceReconciler) deleteWorkspacePod(ctx context.Context, pod *corev
 func (r *WorkspaceReconciler) deleteWorkspaceSecrets(ctx context.Context, ws *workspacev1.Workspace) (err error) {
 	span, ctx := tracing.FromContext(ctx, "deleteWorkspaceSecrets")
 	defer tracing.FinishSpan(span, &err)
-	log := log.FromContext(ctx)
+	log := log.FromContext(ctx).WithValues("owi", ws.OWI())
 
 	// if a secret cannot be deleted we do not return early because we want to attempt
 	// the deletion of the remaining secrets