add custom metrics for RBACSync (#19)

* add custom metrics for RBACSync and fix test

Signed-off-by: Meng-Lin Lu <[email protected]>

Author: mllu

README.md

@@ -234,7 +234,7 @@ debugging.
 To use GSuite, you'll need a service account with "G Suite Domain-Wide
 Delegation of Authority". It's recommended to read the
 [guide](https://developers.google.com/admin-sdk/directory/v1/guides/delegation)
-to understand how this works in cause you run into issues. The blog
+to understand how this works in case you run into issues. The blog
 [Navigating the Google Suite Directory
 API](https://www.fin.com/post/2017/10/navigating-google-suite-directory-api)
 may also provide some insight.

example.yml

@@ -25,7 +25,7 @@ spec:
   - group: someother-group
     roleRef:
       apiGroup: rbac.authorization.k8s.io
-        kind: Role
+      kind: Role
       name: someother-role
 
   # Define group memberships directly

pkg/controller/controller.go

@@ -18,7 +18,6 @@ package controller
 
 import (
 	"fmt"
-	"k8s.io/klog"
 	"time"
 
 	"github.com/pkg/errors"
@@ -41,13 +40,15 @@ import (
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
+	"k8s.io/klog"
 
 	rbacsyncv1alpha "github.com/cruise-automation/rbacsync/pkg/apis/rbacsync/v1alpha"
 	clientset "github.com/cruise-automation/rbacsync/pkg/generated/clientset/versioned"
 	rbacsyncscheme "github.com/cruise-automation/rbacsync/pkg/generated/clientset/versioned/scheme"
 	informers "github.com/cruise-automation/rbacsync/pkg/generated/informers/externalversions/rbacsync/v1alpha"
 	listers "github.com/cruise-automation/rbacsync/pkg/generated/listers/rbacsync/v1alpha"
 	"github.com/cruise-automation/rbacsync/pkg/groups"
+	"github.com/cruise-automation/rbacsync/pkg/metrics"
 )
 
 const (
@@ -58,6 +59,8 @@ const (
 	EventReasonConfigEnqueued    = "ConfigEnqueued"
 	EventReasonBindingConfigured = "BindingConfigured"
 	EventReasonBindingDeleted    = "BindingDeleted"
+	EventReasonBindingDuplicated = "BindingDuplicated"
+	EventReasonBindingWarning    = "BindingWarning"
 	EventReasonBindingError      = "BindingError"
 	EventReasonUnknownGroup      = "UnknownGroup"
 )
@@ -278,8 +281,10 @@ func (c *Controller) enqueue(obj interface{}) {
 	switch obj.(type) {
 	case *rbacsyncv1alpha.RBACSyncConfig:
 		c.queue.AddRateLimited(key)
+		metrics.RBACSyncConfigStatus.WithLabelValues(metrics.LabelKindRBACSyncConfig, EventReasonConfigEnqueued).Inc()
 	case *rbacsyncv1alpha.ClusterRBACSyncConfig:
 		c.clusterqueue.AddRateLimited(key)
+		metrics.RBACSyncConfigStatus.WithLabelValues(metrics.LabelKindClusterRBACSyncConfig, EventReasonConfigEnqueued).Inc()
 	default:
 		klog.Warningf("ignoring object of type %T: %#v", obj, obj)
 		return // skip event emit below
@@ -344,6 +349,7 @@ func (c *Controller) handleConfig(config *rbacsyncv1alpha.RBACSyncConfig) error
 			c.recorder.Eventf(config, corev1.EventTypeWarning,
 				EventReasonBindingError, "RoleRef kind %q invalid for RBACSyncConfig on group %q, use only Role or ClusterRole",
 				binding.RoleRef.Kind, binding.Group)
+			metrics.RBACSyncConfigStatus.WithLabelValues(metrics.LabelKindRBACSyncConfig, EventReasonBindingError).Inc()
 			continue
 		}
 
@@ -354,9 +360,11 @@ func (c *Controller) handleConfig(config *rbacsyncv1alpha.RBACSyncConfig) error
 			if groups.IsNotFound(err) {
 				c.recorder.Eventf(config, corev1.EventTypeWarning,
 					EventReasonUnknownGroup, "group %v not found", binding.Group)
+				metrics.RBACSyncConfigStatus.WithLabelValues(metrics.LabelKindRBACSyncConfig, EventReasonUnknownGroup).Inc()
 			} else if groups.IsUnknownMemberships(err) {
 				c.recorder.Eventf(config, corev1.EventTypeWarning,
 					EventReasonBindingError, "group %v lookup failed: %v", binding.Group, err)
+				metrics.RBACSyncConfigStatus.WithLabelValues(metrics.LabelKindRBACSyncConfig, EventReasonBindingError).Inc()
 				// An error occurred looking up the groups, it should be marked as active
 				// so the rolebindings are not deleted in the cleanup.
 				active[name] = struct{}{}
@@ -367,8 +375,9 @@ func (c *Controller) handleConfig(config *rbacsyncv1alpha.RBACSyncConfig) error
 
 		if len(members) == 0 {
 			c.recorder.Eventf(config, corev1.EventTypeWarning,
-				EventReasonBindingError, "%v/%v has no members for group %v",
+				EventReasonBindingWarning, "%v/%v has no members for group %v",
 				config.Namespace, config.Name, binding.Group)
+			metrics.RBACSyncConfigStatus.WithLabelValues(metrics.LabelKindRBACSyncConfig, EventReasonBindingWarning).Inc()
 			continue
 		}
 
@@ -378,7 +387,8 @@ func (c *Controller) handleConfig(config *rbacsyncv1alpha.RBACSyncConfig) error
 			// result will be the same. Accordingly, we log the bad
 			// configuration and move on.
 			c.recorder.Eventf(config, corev1.EventTypeWarning,
-				EventReasonBindingError, "duplicate binding %v ignored", name)
+				EventReasonBindingDuplicated, "duplicate binding %v ignored", name)
+			metrics.RBACSyncBindingStatus.WithLabelValues(metrics.LabelKindRoleBinding, EventReasonBindingDuplicated).Inc()
 			continue
 		}
 
@@ -403,6 +413,7 @@ func (c *Controller) handleConfig(config *rbacsyncv1alpha.RBACSyncConfig) error
 			c.recorder.Eventf(config, corev1.EventTypeWarning,
 				EventReasonBindingError, "unable to update or create RoleBinding %v/%v: %v",
 				rb.Namespace, rb.Name, err)
+			metrics.RBACSyncBindingStatus.WithLabelValues(metrics.LabelKindRoleBinding, EventReasonBindingError).Inc()
 			continue
 		}
 
@@ -416,7 +427,7 @@ func (c *Controller) handleConfig(config *rbacsyncv1alpha.RBACSyncConfig) error
 		c.recorder.Eventf(config, corev1.EventTypeNormal,
 			EventReasonBindingConfigured,
 			"RoleBinding %v/%v configured", created.Namespace, created.Name)
-
+		metrics.RBACSyncBindingStatus.WithLabelValues(metrics.LabelKindRoleBinding, EventReasonBindingConfigured).Inc()
 	}
 
 	selector, err := buildChildSelector(config.Name)
@@ -451,6 +462,7 @@ func (c *Controller) handleConfig(config *rbacsyncv1alpha.RBACSyncConfig) error
 					EventReasonBindingError,
 					"RoleBinding %v/%v could not be deleted: %v",
 					rb.Namespace, rb.Name, err)
+				metrics.RBACSyncBindingStatus.WithLabelValues(metrics.LabelKindRoleBinding, EventReasonBindingError).Inc()
 				continue
 			}
 
@@ -459,6 +471,7 @@ func (c *Controller) handleConfig(config *rbacsyncv1alpha.RBACSyncConfig) error
 
 		c.recorder.Eventf(config, corev1.EventTypeNormal,
 			EventReasonBindingDeleted, "RoleBinding %v/%v deleted", rb.Namespace, rb.Name)
+		metrics.RBACSyncBindingStatus.WithLabelValues(metrics.LabelKindRoleBinding, EventReasonBindingDeleted).Inc()
 	}
 
 	return nil
@@ -507,6 +520,7 @@ func (c *Controller) handleClusterConfig(config *rbacsyncv1alpha.ClusterRBACSync
 			c.recorder.Eventf(config, corev1.EventTypeWarning,
 				EventReasonBindingError, "RoleRef kind %q invalid for ClusterRBACSyncConfig on group %q, use only ClusterRole",
 				binding.RoleRef.Kind, binding.Group)
+			metrics.RBACSyncConfigStatus.WithLabelValues(metrics.LabelKindClusterRBACSyncConfig, EventReasonBindingError).Inc()
 			continue
 		}
 		name := config.Name + "-" + binding.Group + "-" + binding.RoleRef.Name
@@ -516,9 +530,11 @@ func (c *Controller) handleClusterConfig(config *rbacsyncv1alpha.ClusterRBACSync
 			if groups.IsNotFound(err) {
 				c.recorder.Eventf(config, corev1.EventTypeWarning,
 					EventReasonUnknownGroup, "group %v not found", binding.Group)
+				metrics.RBACSyncConfigStatus.WithLabelValues(metrics.LabelKindClusterRBACSyncConfig, EventReasonUnknownGroup).Inc()
 			} else if groups.IsUnknownMemberships(err) {
 				c.recorder.Eventf(config, corev1.EventTypeWarning,
 					EventReasonBindingError, "group %v lookup failed: %v", binding.Group, err)
+				metrics.RBACSyncConfigStatus.WithLabelValues(metrics.LabelKindClusterRBACSyncConfig, EventReasonBindingError).Inc()
 				// In the case of unknown memberships from the grouper, we want to keep what already exists
 				// so we mark the binding as active.
 				active[name] = struct{}{}
@@ -529,8 +545,9 @@ func (c *Controller) handleClusterConfig(config *rbacsyncv1alpha.ClusterRBACSync
 
 		if len(members) == 0 {
 			c.recorder.Eventf(config, corev1.EventTypeWarning,
-				EventReasonBindingError, "%v has no members for group %v",
+				EventReasonBindingWarning, "%v has no members for group %v",
 				config.Name, binding.Group)
+			metrics.RBACSyncConfigStatus.WithLabelValues(metrics.LabelKindClusterRBACSyncConfig, EventReasonBindingWarning).Inc()
 			continue
 		}
 
@@ -540,7 +557,8 @@ func (c *Controller) handleClusterConfig(config *rbacsyncv1alpha.ClusterRBACSync
 			// result will be the same. Accordingly, we log the bad
 			// configuration and move on.
 			c.recorder.Eventf(config, corev1.EventTypeWarning,
-				EventReasonBindingError, "duplicate binding %v ignored", name)
+				EventReasonBindingDuplicated, "duplicate binding %v ignored", name)
+			metrics.RBACSyncBindingStatus.WithLabelValues(metrics.LabelKindClusterRoleBinding, EventReasonBindingDuplicated).Inc()
 			continue
 		}
 
@@ -564,6 +582,7 @@ func (c *Controller) handleClusterConfig(config *rbacsyncv1alpha.ClusterRBACSync
 			c.recorder.Eventf(config, corev1.EventTypeWarning,
 				EventReasonBindingError,
 				"unable to update or create ClusterRoleBinding %v: %v", crb.Name, err)
+			metrics.RBACSyncBindingStatus.WithLabelValues(metrics.LabelKindClusterRoleBinding, EventReasonBindingError).Inc()
 			continue
 		}
 
@@ -577,6 +596,7 @@ func (c *Controller) handleClusterConfig(config *rbacsyncv1alpha.ClusterRBACSync
 		c.recorder.Eventf(config, corev1.EventTypeNormal,
 			EventReasonBindingConfigured,
 			"ClusterRoleBinding %v configured", created.Name)
+		metrics.RBACSyncBindingStatus.WithLabelValues(metrics.LabelKindClusterRoleBinding, EventReasonBindingConfigured).Inc()
 	}
 
 	selector, err := buildChildSelector(config.Name)
@@ -609,6 +629,7 @@ func (c *Controller) handleClusterConfig(config *rbacsyncv1alpha.ClusterRBACSync
 				c.recorder.Eventf(config, corev1.EventTypeWarning,
 					EventReasonBindingError,
 					"ClusterRoleBinding %v could not be deleted: %v", crb.Name, err)
+				metrics.RBACSyncBindingStatus.WithLabelValues(metrics.LabelKindClusterRoleBinding, EventReasonBindingError).Inc()
 				continue
 			}
 
@@ -617,6 +638,7 @@ func (c *Controller) handleClusterConfig(config *rbacsyncv1alpha.ClusterRBACSync
 
 		c.recorder.Eventf(config, corev1.EventTypeNormal,
 			EventReasonBindingDeleted, "ClusterRoleBinding %v deleted", crb.Name)
+		metrics.RBACSyncBindingStatus.WithLabelValues(metrics.LabelKindClusterRoleBinding, EventReasonBindingDeleted).Inc()
 	}
 
 	return nil

pkg/controller/controller_test.go

@@ -18,11 +18,12 @@ package controller
 
 import (
 	"fmt"
-	"github.com/pkg/errors"
-	"k8s.io/klog"
 	"testing"
 	"time"
 
+	"github.com/pkg/errors"
+	"k8s.io/klog"
+
 	rbacsyncv1alpha "github.com/cruise-automation/rbacsync/pkg/apis/rbacsync/v1alpha"
 	"github.com/cruise-automation/rbacsync/pkg/checks"
 	rsfake "github.com/cruise-automation/rbacsync/pkg/generated/clientset/versioned/fake"
@@ -116,7 +117,7 @@ func TestControllerRBACSyncConfig(t *testing.T) {
 				// the controller. It just creates the binding twice. We may
 				// change this to avoid extra round trip in these cases. We
 				// should probably actually warn.
-				"Warning BindingError duplicate binding duplicates-group0-role0 ignored",
+				"Warning BindingDuplicated duplicate binding duplicates-group0-role0 ignored",
 				"Normal BindingConfigured RoleBinding testing/duplicates-group0-role1 configured",
 				"Normal BindingConfigured RoleBinding testing/duplicates-upstream-role0 configured",
 			},
@@ -256,7 +257,7 @@ func TestControllerClusterRBACSyncConfig(t *testing.T) {
 				// the controller. It just creates the binding twice. We may
 				// change this to avoid extra round trip in these cases. We
 				// should probably actually warn.
-				"Warning BindingError duplicate binding duplicates-group0-role0 ignored",
+				"Warning BindingDuplicated duplicate binding duplicates-group0-role0 ignored",
 				"Normal BindingConfigured ClusterRoleBinding duplicates-group0-role1 configured",
 				"Normal BindingConfigured ClusterRoleBinding duplicates-upstream-role0 configured",
 			},

pkg/groups/gsuite/grouper.go

@@ -23,13 +23,15 @@ import (
 	"regexp"
 	"time"
 
-	"github.com/cruise-automation/rbacsync/pkg/groups"
 	"github.com/pkg/errors"
 	"golang.org/x/oauth2/google"
 	"golang.org/x/oauth2/jwt"
 	admin "google.golang.org/api/admin/directory/v1"
 	"google.golang.org/api/googleapi"
 	rbacv1 "k8s.io/api/rbac/v1"
+
+	"github.com/cruise-automation/rbacsync/pkg/groups"
+	"github.com/cruise-automation/rbacsync/pkg/metrics"
 )
 
 const (
@@ -109,17 +111,20 @@ func (g *Grouper) Members(group string) ([]rbacv1.Subject, error) {
 	ctx := context.TODO()
 	client, err := g.service(ctx)
 	if err != nil {
+		metrics.RBACSyncGsuiteClientCreationStatus.WithLabelValues("Failed").Inc()
 		return nil, errors.Wrapf(groups.ErrUnknown,
-		"unable to determine group members, an error occurred creating gsuite client: %v",
-		err)
+			"unable to determine group members, an error occurred creating gsuite client: %v",
+			err)
 	}
+	metrics.RBACSyncGsuiteClientCreationStatus.WithLabelValues("Succeeded").Inc()
 
 	var (
 		tctx, cancel = context.WithTimeout(ctx, g.timeout)
 		subjects     []rbacv1.Subject
 	)
 	defer cancel()
 
+	startTime := time.Now()
 	if err := client.Members.List(group).
 		IncludeDerivedMembership(true).
 		Pages(tctx, func(members *admin.Members) error {
@@ -135,16 +140,36 @@ func (g *Grouper) Members(group string) ([]rbacv1.Subject, error) {
 
 		switch {
 		case isNotFound(err):
+			metrics.RBACSyncGsuiteMembersLatency.WithLabelValues("NotFound").Observe(
+				float64(time.Since(startTime).Nanoseconds()) / float64(time.Second),
+			)
+			metrics.RBACSyncGsuiteMembersStatus.WithLabelValues("NotFound").Inc()
 			return nil, errors.Wrapf(groups.ErrNotFound, "gsuite does not have group: %v", err)
 		case isTimeout(tctx):
+			metrics.RBACSyncGsuiteMembersLatency.WithLabelValues("Timeout").Observe(
+				float64(time.Since(startTime).Nanoseconds()) / float64(time.Second),
+			)
+			metrics.RBACSyncGsuiteMembersStatus.WithLabelValues("Timeout").Inc()
 			return nil, errors.Wrapf(groups.ErrTimeout, "timeout calling gsuite api: %v", err)
 		case isCanceled(tctx):
+			metrics.RBACSyncGsuiteMembersLatency.WithLabelValues("Canceled").Observe(
+				float64(time.Since(startTime).Nanoseconds()) / float64(time.Second),
+			)
+			metrics.RBACSyncGsuiteMembersStatus.WithLabelValues("Canceled").Inc()
 			return nil, errors.Wrapf(groups.ErrCanceled, "the context canceled the call to gsuite: %v", err)
 		default:
+			metrics.RBACSyncGsuiteMembersLatency.WithLabelValues("Unknown").Observe(
+				float64(time.Since(startTime).Nanoseconds()) / float64(time.Second),
+			)
+			metrics.RBACSyncGsuiteMembersStatus.WithLabelValues("Unknown").Inc()
 			return nil, errors.Wrapf(groups.ErrUnknown, "error retrieving group members: %v", err)
 		}
 
 	}
+	metrics.RBACSyncGsuiteMembersLatency.WithLabelValues("Succeeded").Observe(
+		float64(time.Since(startTime).Nanoseconds()) / float64(time.Second),
+	)
+	metrics.RBACSyncGsuiteMembersStatus.WithLabelValues("Succeeded").Inc()
 
 	// If you're trying to find some nasty memory allocation, it might be here.
 	// Grouper interface should be converted to callback style if this is a

pkg/metrics/metrics.go

@@ -0,0 +1,40 @@
+package metrics
+
+import (
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"
+)
+
+const (
+	LabelKindRBACSyncConfig        = "RBACSyncConfig"
+	LabelKindClusterRBACSyncConfig = "ClusterRBACSyncConfig"
+
+	LabelKindRoleBinding        = "RoleBinding"
+	LabelKindClusterRoleBinding = "ClusterRoleBinding"
+)
+
+var (
+	// Metrics for Controller
+	RBACSyncConfigStatus = promauto.NewGaugeVec(prometheus.GaugeOpts{
+		Name: "rbacsync_config_status",
+		Help: "The number of RBACSyncConfigs and RBACSyncClusterConfigs and the status of the processed config",
+	}, []string{"kind", "status"})
+	RBACSyncBindingStatus = promauto.NewGaugeVec(prometheus.GaugeOpts{
+		Name: "rbacsync_binding_status",
+		Help: "The number of RoleBindings and ClusterRoleBindings configured by the controller and their statuses",
+	}, []string{"kind", "status"})
+
+	// Metrics for Mapper/GSuite
+	RBACSyncGsuiteClientCreationStatus = promauto.NewCounterVec(prometheus.CounterOpts{
+		Name: "rbacsync_gsuite_client_creation_status",
+		Help: "Total number of the status of gsuite client creations",
+	}, []string{"status"})
+	RBACSyncGsuiteMembersStatus = promauto.NewCounterVec(prometheus.CounterOpts{
+		Name: "rbacsync_gsuite_members_status",
+		Help: "Total number of the status of calls to gsuite with labels for state",
+	}, []string{"status"})
+	RBACSyncGsuiteMembersLatency = promauto.NewHistogramVec(prometheus.HistogramOpts{
+		Name: "rbacsync_gsuite_members_latency_duration_seconds",
+		Help: "The amount of time the calls to gsuite for group memberships",
+	}, []string{"status"})
+)