[ws-manager-mk2] Switch to lru cache

Author: Furisto

components/ws-daemon/pkg/controller/controller.go

@@ -204,7 +204,7 @@ func (wsc *WorkspaceController) handleWorkspaceInit(ctx context.Context, ws *wor
 			return wsc.Status().Update(ctx, ws)
 		})
 
-		if err != nil {
+		if err == nil {
 			wsc.metrics.recordInitializeTime(time.Since(initStart).Seconds(), ws)
 		}
 
@@ -277,7 +277,7 @@ func (wsc *WorkspaceController) handleWorkspaceStop(ctx context.Context, ws *wor
 		return wsc.Status().Update(ctx, ws)
 	})
 
-	if err != nil {
+	if err == nil {
 		wsc.metrics.recordFinalizeTime(time.Since(disposeStart).Seconds(), ws)
 	}
 

components/ws-manager-api/go/crd/v1/workspace_types.go

@@ -128,9 +128,6 @@ type WorkspaceStatus struct {
 
 	// +kubebuilder:validation:Optional
 	Runtime *WorkspaceRuntimeStatus `json:"runtime,omitempty"`
-
-	// +kubebuilder:validation:Optional
-	Metrics map[string]bool `json:"metrics,omitempty"`
 }
 
 // +kubebuilder:validation:Enum=Deployed;Failed;Timeout;UserActivity;HeadlessTaskFailed;StoppedByRequest;EverReady;ContentReady;BackupComplete;BackupFailure

components/ws-manager-mk2/config/crd/bases/workspace.gitpod.io_workspaces.yaml

@@ -395,10 +395,6 @@ spec:
                 type: object
               headless:
                 type: boolean
-              metrics:
-                additionalProperties:
-                  type: boolean
-                type: object
               ownerToken:
                 type: string
               phase:

components/ws-manager-mk2/controllers/metrics.go

@@ -11,6 +11,7 @@ import (
 
 	workspacev1 "github.com/gitpod-io/gitpod/ws-manager/api/crd/v1"
 	"github.com/go-logr/logr"
+	lru "github.com/hashicorp/golang-lru"
 	"github.com/prometheus/client_golang/prometheus"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -37,9 +38,17 @@ type controllerMetrics struct {
 
 	workspacePhases *phaseTotalVec
 	timeoutSettings *timeoutSettingsVec
+
+	// used to prevent recording metrics multiple times
+	cache *lru.Cache
 }
 
-func newControllerMetrics(r *WorkspaceReconciler) *controllerMetrics {
+func newControllerMetrics(r *WorkspaceReconciler) (*controllerMetrics, error) {
+	cache, err := lru.New(6000)
+	if err != nil {
+		return nil, err
+	}
+
 	return &controllerMetrics{
 		startupTimeHistVec: prometheus.NewHistogramVec(prometheus.HistogramOpts{
 			Namespace: metricsNamespace,
@@ -88,14 +97,11 @@ func newControllerMetrics(r *WorkspaceReconciler) *controllerMetrics {
 
 		workspacePhases: newPhaseTotalVec(r),
 		timeoutSettings: newTimeoutSettingsVec(r),
-	}
+		cache:           cache,
+	}, nil
 }
 
 func (m *controllerMetrics) recordWorkspaceStartupTime(log *logr.Logger, ws *workspacev1.Workspace) {
-	if _, countedAlready := ws.Status.Metrics[workspaceStartupSeconds]; countedAlready {
-		return
-	}
-
 	class := ws.Spec.Class
 	tpe := string(ws.Spec.Type)
 
@@ -105,14 +111,9 @@ func (m *controllerMetrics) recordWorkspaceStartupTime(log *logr.Logger, ws *wor
 	}
 
 	hist.Observe(float64(time.Since(ws.CreationTimestamp.Time).Seconds()))
-	ws.Status.Metrics[workspaceStartupSeconds] = true
 }
 
 func (m *controllerMetrics) countWorkspaceStartFailures(log *logr.Logger, ws *workspacev1.Workspace) {
-	if _, countedAlready := ws.Status.Metrics[workspaceStartFailuresTotal]; countedAlready {
-		return
-	}
-
 	class := ws.Spec.Class
 	tpe := string(ws.Spec.Type)
 
@@ -122,14 +123,9 @@ func (m *controllerMetrics) countWorkspaceStartFailures(log *logr.Logger, ws *wo
 	}
 
 	counter.Inc()
-	ws.Status.Metrics[workspaceStartFailuresTotal] = true
 }
 
 func (m *controllerMetrics) countWorkspaceStop(log *logr.Logger, ws *workspacev1.Workspace) {
-	if _, countedAlready := ws.Status.Metrics[workspaceStopsTotal]; countedAlready {
-		return
-	}
-
 	class := ws.Spec.Class
 	tpe := string(ws.Spec.Type)
 
@@ -139,14 +135,9 @@ func (m *controllerMetrics) countWorkspaceStop(log *logr.Logger, ws *workspacev1
 	}
 
 	counter.Inc()
-	ws.Status.Metrics[workspaceStopsTotal] = true
 }
 
 func (m *controllerMetrics) countTotalBackups(log *logr.Logger, ws *workspacev1.Workspace) {
-	if _, countedAlready := ws.Status.Metrics[workspaceBackupsTotal]; countedAlready {
-		return
-	}
-
 	class := ws.Spec.Class
 	tpe := string(ws.Spec.Type)
 
@@ -156,14 +147,9 @@ func (m *controllerMetrics) countTotalBackups(log *logr.Logger, ws *workspacev1.
 	}
 
 	counter.Inc()
-	ws.Status.Metrics[workspaceBackupsTotal] = true
 }
 
 func (m *controllerMetrics) countTotalBackupFailures(log *logr.Logger, ws *workspacev1.Workspace) {
-	if _, countedAlready := ws.Status.Metrics[workspaceBackupFailuresTotal]; countedAlready {
-		return
-	}
-
 	class := ws.Spec.Class
 	tpe := string(ws.Spec.Type)
 
@@ -173,14 +159,9 @@ func (m *controllerMetrics) countTotalBackupFailures(log *logr.Logger, ws *works
 	}
 
 	counter.Inc()
-	ws.Status.Metrics[workspaceBackupFailuresTotal] = true
 }
 
 func (m *controllerMetrics) countTotalRestores(log *logr.Logger, ws *workspacev1.Workspace) {
-	if _, countedAlready := ws.Status.Metrics[workspaceRestoresTotal]; countedAlready {
-		return
-	}
-
 	class := ws.Spec.Class
 	tpe := string(ws.Spec.Type)
 
@@ -190,14 +171,9 @@ func (m *controllerMetrics) countTotalRestores(log *logr.Logger, ws *workspacev1
 	}
 
 	counter.Inc()
-	ws.Status.Metrics[workspaceRestoresTotal] = true
 }
 
 func (m *controllerMetrics) countTotalRestoreFailures(log *logr.Logger, ws *workspacev1.Workspace) {
-	if _, countedAlready := ws.Status.Metrics[workspaceRestoresFailureTotal]; countedAlready {
-		return
-	}
-
 	class := ws.Spec.Class
 	tpe := string(ws.Spec.Type)
 
@@ -207,7 +183,24 @@ func (m *controllerMetrics) countTotalRestoreFailures(log *logr.Logger, ws *work
 	}
 
 	counter.Inc()
-	ws.Status.Metrics[workspaceRestoresFailureTotal] = true
+}
+
+func (m *controllerMetrics) rememberWorkspace(ws *workspacev1.Workspace) {
+	m.cache.Add(ws.Name, ws.Status.Phase)
+}
+
+func (m *controllerMetrics) forgetWorkspace(ws *workspacev1.Workspace) {
+	m.cache.Remove(ws.Name)
+}
+
+func (m *controllerMetrics) shouldUpdate(log *logr.Logger, ws *workspacev1.Workspace) bool {
+	p, ok := m.cache.Get(ws.Name)
+	if !ok {
+		return false
+	}
+
+	phase := p.(workspacev1.WorkspacePhase)
+	return phase != ws.Status.Phase
 }
 
 // Describe implements Collector. It will send exactly one Desc to the provided channel.

components/ws-manager-mk2/controllers/workspace_controller.go

@@ -35,7 +35,10 @@ func NewWorkspaceReconciler(c client.Client, scheme *runtime.Scheme, cfg config.
 		Config: cfg,
 	}
 
-	metrics := newControllerMetrics(reconciler)
+	metrics, err := newControllerMetrics(reconciler)
+	if err != nil {
+		return nil, err
+	}
 	reg.MustRegister(metrics)
 	reconciler.metrics = metrics
 
@@ -84,10 +87,6 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		workspace.Status.Conditions = []metav1.Condition{}
 	}
 
-	if workspace.Status.Metrics == nil {
-		workspace.Status.Metrics = make(map[string]bool)
-	}
-
 	log.Info("reconciling workspace", "ws", req.NamespacedName)
 
 	var workspacePods corev1.PodList
@@ -156,6 +155,7 @@ func (r *WorkspaceReconciler) actOnStatus(ctx context.Context, workspace *worksp
 				//			 need to be deleted and re-created
 				workspace.Status.PodStarts++
 			}
+			r.metrics.rememberWorkspace(workspace)
 
 		case workspace.Status.Phase == workspacev1.WorkspacePhaseStopped:
 			err := r.Client.Delete(ctx, workspace)
@@ -233,6 +233,10 @@ func (r *WorkspaceReconciler) updateMetrics(ctx context.Context, workspace *work
 
 	phase := workspace.Status.Phase
 
+	if !r.metrics.shouldUpdate(&log, workspace) {
+		return
+	}
+
 	switch {
 	case phase == workspacev1.WorkspacePhasePending ||
 		phase == workspacev1.WorkspacePhaseCreating ||
@@ -264,7 +268,11 @@ func (r *WorkspaceReconciler) updateMetrics(ctx context.Context, workspace *work
 		}
 
 		r.metrics.countWorkspaceStop(&log, workspace)
+		r.metrics.forgetWorkspace(workspace)
+		return
 	}
+
+	r.metrics.rememberWorkspace(workspace)
 }
 
 func conditionPresentAndTrue(cond []metav1.Condition, tpe string) bool {