Bug 1822396: Delete subscription metric when an operator is uninstalled

When an operator was subscribed to using a Subscription Object, the subscription_sync_total
metric was emitted whenever the Subscription Object was created/updated/deleted. This PR
updates that behaviour to emit the metric only when the Subscription object is created/updated,
and deletes the time series for that particular subscription when the subscription object is
deleted.

Author: anik120

pkg/controller/operators/catalog/subscription/syncer.go

@@ -45,34 +45,37 @@ func (s *subscriptionSyncer) now() *metav1.Time {
 // Sync reconciles Subscription events by invoking a sequence of reconcilers, passing the result of each
 // successful reconciliation as an argument to its successor.
 func (s *subscriptionSyncer) Sync(ctx context.Context, event kubestate.ResourceEvent) error {
-	res := &v1alpha1.Subscription{}
-	if err := scheme.Convert(event.Resource(), res, nil); err != nil {
+	sub := &v1alpha1.Subscription{}
+	if err := scheme.Convert(event.Resource(), sub, nil); err != nil {
 		return err
 	}
 
-	s.recordMetrics(res)
+	metrics.EmitSubMetric(sub)
 
 	logger := s.logger.WithFields(logrus.Fields{
-		"reconciling": fmt.Sprintf("%T", res),
-		"selflink":    res.GetSelfLink(),
+		"reconciling": fmt.Sprintf("%T", sub),
+		"selflink":    sub.GetSelfLink(),
 		"event":       event.Type(),
 	})
 	logger.Info("syncing")
 
 	// Enter initial state based on subscription and event type
 	// TODO: Consider generalizing initial generic add, update, delete transitions in the kubestate package.
 	// 		 Possibly make a resource event aware bridge between Sync and reconciler.
-	initial := NewSubscriptionState(res.DeepCopy())
+	initialSubState := NewSubscriptionState(sub.DeepCopy())
 	switch event.Type() {
 	case kubestate.ResourceAdded:
-		initial = initial.Add()
+		initialSubState = initialSubState.Add()
 	case kubestate.ResourceUpdated:
-		initial = initial.Update()
+		initialSubState = initialSubState.Update()
 	case kubestate.ResourceDeleted:
-		initial = initial.Delete()
+		{
+			initialSubState = initialSubState.Delete()
+			metrics.DeleteSubsMetric(sub)
+		}
 	}
 
-	reconciled, err := s.reconcilers.Reconcile(ctx, initial)
+	reconciled, err := s.reconcilers.Reconcile(ctx, initialSubState)
 	if err != nil {
 		logger.WithError(err).Warn("an error was encountered during reconciliation")
 		return err
@@ -85,15 +88,6 @@ func (s *subscriptionSyncer) Sync(ctx context.Context, event kubestate.ResourceE
 	return nil
 }
 
-func (s *subscriptionSyncer) recordMetrics(sub *v1alpha1.Subscription) {
-	// sub.Spec is not a required field.
-	if sub.Spec == nil {
-		return
-	}
-
-	metrics.CounterForSubscription(sub.GetName(), sub.Status.InstalledCSV, sub.Spec.Channel, sub.Spec.Package).Inc()
-}
-
 func (s *subscriptionSyncer) Notify(event kubestate.ResourceEvent) {
 	s.notify(event)
 }

pkg/metrics/metrics.go

@@ -2,6 +2,7 @@ package metrics
 
 import (
 	"context"
+
 	"github.com/prometheus/client_golang/prometheus"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -217,3 +218,11 @@ func EmitCSVMetric(oldCSV *olmv1alpha1.ClusterServiceVersion, newCSV *olmv1alpha
 		csvAbnormal.WithLabelValues(newCSV.Namespace, newCSV.Name, newCSV.Spec.Version.String(), string(newCSV.Status.Phase), string(newCSV.Status.Reason)).Set(1)
 	}
 }
+
+func EmitSubMetric(sub *olmv1alpha1.Subscription) {
+	SubscriptionSyncCount.WithLabelValues(sub.GetName(), sub.Status.InstalledCSV, sub.Spec.Channel, sub.Spec.Package).Inc()
+}
+
+func DeleteSubsMetric(sub *olmv1alpha1.Subscription) {
+	SubscriptionSyncCount.DeleteLabelValues(sub.GetName(), sub.Status.InstalledCSV, sub.Spec.Channel, sub.Spec.Package)
+}