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 {
+	subs := &v1alpha1.Subscription{}
+	if err := scheme.Convert(event.Resource(), subs, nil); err != nil {
 		return err
 	}
 
-	s.recordMetrics(res)
+	metrics.EmitSubsMetrics(subs)
 
 	logger := s.logger.WithFields(logrus.Fields{
-		"reconciling": fmt.Sprintf("%T", res),
-		"selflink":    res.GetSelfLink(),
+		"reconciling": fmt.Sprintf("%T", subs),
+		"selflink":    subs.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())
+	initialSubsState := NewSubscriptionState(subs.DeepCopy())
 	switch event.Type() {
 	case kubestate.ResourceAdded:
-		initial = initial.Add()
+		initialSubsState = initialSubsState.Add()
 	case kubestate.ResourceUpdated:
-		initial = initial.Update()
+		initialSubsState = initialSubsState.Update()
 	case kubestate.ResourceDeleted:
-		initial = initial.Delete()
+		{
+			initialSubsState = initialSubsState.Delete()
+			metrics.DeleteSubsMetric(subs)
+		}
 	}
 
-	reconciled, err := s.reconcilers.Reconcile(ctx, initial)
+	reconciled, err := s.reconcilers.Reconcile(ctx, initialSubsState)
 	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

@@ -185,10 +185,6 @@ func RegisterCatalog() {
 	prometheus.MustRegister(SubscriptionSyncCount)
 }
 
-func CounterForSubscription(name, installedCSV, channelName, packageName string) prometheus.Counter {
-	return SubscriptionSyncCount.WithLabelValues(name, installedCSV, channelName, packageName)
-}
-
 func EmitCSVMetric(oldCSV *olmv1alpha1.ClusterServiceVersion, newCSV *olmv1alpha1.ClusterServiceVersion) {
 	if oldCSV == nil || newCSV == nil {
 		return
@@ -212,3 +208,16 @@ 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 EmitSubsMetrics(sub *olmv1alpha1.Subscription) {
+	// sub.Spec is not a required field.
+	if sub.Spec == nil {
+		return
+	}
+
+	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)
+}