fix: correct fingerprinting and common enhancer call (#78)

* fix fingerprinting and common enhancer call

* refactor enhancers

Author: Jiahui-Zhang-20

enhancers.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 
 	"github.com/getsentry/sentry-go"
+	"github.com/rs/zerolog"
 	appsv1 "k8s.io/api/apps/v1"
 	batchv1 "k8s.io/api/batch/v1"
 	v1 "k8s.io/api/core/v1"
@@ -16,17 +17,19 @@ const breadcrumbLimit = 20
 
 func runEnhancers(ctx context.Context, eventObject *v1.Event, kind string, object metav1.Object, scope *sentry.Scope, sentryEvent *sentry.Event) error {
 
-	involvedObject := fmt.Sprintf("%s/%s", kind, object.GetName())
-	ctx, logger := getLoggerWithTag(ctx, "object", involvedObject)
+	logger := zerolog.Ctx(ctx)
 	logger.Debug().Msgf("Running the enhancer")
 
 	var err error
 
-	// First, run the common enhancer
+	// First, run the common enhancer which
+	// does not need neither the event object
+	// nor the involved object
 	err = runCommonEnhancer(ctx, scope, sentryEvent)
 	if err != nil {
 		return err
 	}
+
 	// If an event object is provided, we call the event enhancer
 	if eventObject != nil {
 		err = eventEnhancer(ctx, scope, eventObject, sentryEvent)
@@ -35,42 +38,17 @@ func runEnhancers(ctx context.Context, eventObject *v1.Event, kind string, objec
 		}
 	}
 
-	logger.Trace().Msgf("Current fingerprint: %v", sentryEvent.Fingerprint)
-	// Enhance message with object name
-	message := sentryEvent.Message
-	sentryEvent.Message = fmt.Sprintf("%s: %s", object.GetName(), sentryEvent.Message)
-
-	// Adjust fingerprint.
-	// If there's already a non-empty fingerprint set, we assume that it was set by
-	// another enhancer, so we don't touch it.
-	if len(sentryEvent.Fingerprint) == 0 {
-		sentryEvent.Fingerprint = []string{message}
-	}
-	logger.Trace().Msgf("Fingerprint after adjustment: %v", sentryEvent.Fingerprint)
-
-	// Find the root owners and their corresponding object kinds
-	rootOwners, err := findRootOwners(ctx, &KindObjectPair{
-		kind:   kind,
-		object: object,
-	})
-	if err != nil {
-		return err
-	}
-
-	// Call the specific enhancer for the object
-	callObjectEnhancer(ctx, scope, &KindObjectPair{
-		kind,
-		object,
-	}, sentryEvent)
-
-	// Call specific enhancers for all root owners
-	// (there most likely is just one root owner)
-	for _, rootOwner := range rootOwners {
-		callObjectEnhancer(ctx, scope, &rootOwner, sentryEvent)
+	// If an involved object is provided, we call the object enhancer
+	if object != nil {
+		err = objectEnhancer(ctx, scope, &KindObjectPair{
+			kind:   kind,
+			object: object,
+		}, sentryEvent)
 		if err != nil {
 			return err
 		}
 	}
+
 	return nil
 }
 
@@ -79,76 +57,6 @@ type KindObjectPair struct {
 	object metav1.Object
 }
 
-func findRootOwners(ctx context.Context, kindObjPair *KindObjectPair) ([]KindObjectPair, error) {
-
-	// use DFS to find the leaves of the owner references graph
-	rootOwners, err := ownerRefDFS(ctx, kindObjPair)
-	if err != nil {
-		return nil, err
-	}
-
-	// if the object has no owner references
-	if rootOwners[0].object.GetUID() == kindObjPair.object.GetUID() {
-		return []KindObjectPair{}, nil
-	}
-
-	return rootOwners, nil
-
-}
-
-// this function finds performs DFS to find the leaves the owner references graph
-func ownerRefDFS(ctx context.Context, kindObjPair *KindObjectPair) ([]KindObjectPair, error) {
-
-	parents := kindObjPair.object.GetOwnerReferences()
-	// the owners slice to be returned
-	rootOwners := []KindObjectPair{}
-
-	// base case: the object has no parents
-	if len(parents) == 0 {
-		rootOwners = append(rootOwners, *kindObjPair)
-		return rootOwners, nil
-	}
-
-	// recursive case: the object has parents to explore
-	for _, parent := range parents {
-		parentObj, ok := findObject(ctx, parent.Kind, kindObjPair.object.GetNamespace(), parent.Name)
-		if !ok {
-			return nil, errors.New("error attempting to find root owneres")
-		}
-		partialOwners, err := ownerRefDFS(ctx, &KindObjectPair{
-			kind:   parent.Kind,
-			object: parentObj,
-		})
-		if err != nil {
-			return nil, err
-		}
-		if partialOwners != nil {
-			rootOwners = append(rootOwners, partialOwners...)
-		}
-	}
-	return rootOwners, nil
-}
-
-func callObjectEnhancer(ctx context.Context, scope *sentry.Scope, kindObjectPair *KindObjectPair, sentryEvent *sentry.Event) error {
-
-	var err error = nil
-	switch kindObjectPair.kind {
-	case KindPod:
-		err = podEnhancer(ctx, scope, kindObjectPair.object, sentryEvent)
-	case KindReplicaset:
-		err = replicaSetEnhancer(ctx, scope, kindObjectPair.object, sentryEvent)
-	case KindDeployment:
-		err = deploymentEnhancer(ctx, scope, kindObjectPair.object, sentryEvent)
-	case KindJob:
-		err = jobEnhancer(ctx, scope, kindObjectPair.object, sentryEvent)
-	case KindCronjob:
-		err = cronjobEnhancer(ctx, scope, kindObjectPair.object, sentryEvent)
-	default:
-		sentryEvent.Fingerprint = append(sentryEvent.Fingerprint, kindObjectPair.object.GetName())
-	}
-	return err
-}
-
 func eventEnhancer(ctx context.Context, scope *sentry.Scope, object metav1.Object, sentryEvent *sentry.Event) error {
 	eventObj, ok := object.(*v1.Event)
 	if !ok {
@@ -179,6 +87,79 @@ func eventEnhancer(ctx context.Context, scope *sentry.Scope, object metav1.Objec
 	return nil
 }
 
+func objectEnhancer(ctx context.Context, scope *sentry.Scope, kindObjectPair *KindObjectPair, sentryEvent *sentry.Event) error {
+
+	objectTag := fmt.Sprintf("%s/%s", kindObjectPair.kind, kindObjectPair.object.GetName())
+	ctx, logger := getLoggerWithTag(ctx, "object", objectTag)
+
+	var err error = nil
+
+	logger.Trace().Msgf("Current fingerprint: %v", sentryEvent.Fingerprint)
+
+	// Enhance message with object name
+	message := sentryEvent.Message
+	sentryEvent.Message = fmt.Sprintf("%s: %s", kindObjectPair.object.GetName(), sentryEvent.Message)
+
+	// Adjust fingerprint.
+	// If there's already a non-empty fingerprint set, we assume that it was set by
+	// another enhancer, so we don't touch it.
+	if len(sentryEvent.Fingerprint) == 0 {
+		sentryEvent.Fingerprint = []string{message}
+	}
+	logger.Trace().Msgf("Fingerprint after adjustment: %v", sentryEvent.Fingerprint)
+
+	// Find the root owners and their corresponding object kinds
+	rootOwners, err := findRootOwners(ctx, kindObjectPair)
+	if err != nil {
+		return err
+	}
+
+	// Might reset back to old fingerprint if
+	// there exists root owner(s) to the object
+	oldFingerprint := sentryEvent.Fingerprint
+	// Call the specific enhancer for the object
+	err = getKindEnhancer(kindObjectPair.kind)(ctx, scope, kindObjectPair.object, sentryEvent)
+	if err != nil {
+		return err
+	}
+	// Remove any fingerprinting so the event
+	// can be grouped by its owners instead
+	if len(rootOwners) != 0 {
+		sentryEvent.Fingerprint = oldFingerprint
+	}
+
+	// Call specific enhancers for all root owners
+	// (there most likely is just one root owner)
+	for _, rootOwner := range rootOwners {
+		err = getKindEnhancer(rootOwner.kind)(ctx, scope, rootOwner.object, sentryEvent)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func getKindEnhancer(kind string) func(context.Context, *sentry.Scope, metav1.Object, *sentry.Event) error {
+	switch kind {
+	case KindPod:
+		return podEnhancer
+	case KindReplicaset:
+		return replicaSetEnhancer
+	case KindDeployment:
+		return deploymentEnhancer
+	case KindJob:
+		return jobEnhancer
+	case KindCronjob:
+		return cronjobEnhancer
+	default:
+		return func(ctx context.Context, scope *sentry.Scope, object metav1.Object, sentryEvent *sentry.Event) error {
+			sentryEvent.Fingerprint = append(sentryEvent.Fingerprint, object.GetName())
+			return nil
+		}
+	}
+}
+
 func podEnhancer(ctx context.Context, scope *sentry.Scope, object metav1.Object, sentryEvent *sentry.Event) error {
 	podObj, ok := object.(*v1.Pod)
 	if !ok {
@@ -189,10 +170,10 @@ func podEnhancer(ctx context.Context, scope *sentry.Scope, object metav1.Object,
 	nodeName := podObj.Spec.NodeName
 	setTagIfNotEmpty(scope, "node_name", nodeName)
 
-	// Add the cronjob to the fingerprint
+	// Add the pod name to the fingerprint
 	sentryEvent.Fingerprint = append(sentryEvent.Fingerprint, KindPod, podObj.Name)
 
-	// Add the cronjob to the tag
+	// Add the pod to the tag
 	setTagIfNotEmpty(scope, "pod_name", object.GetName())
 	podObj.ManagedFields = []metav1.ManagedFieldsEntry{}
 	metadataJson, err := prettyJson(podObj.ObjectMeta)
@@ -202,7 +183,7 @@ func podEnhancer(ctx context.Context, scope *sentry.Scope, object metav1.Object,
 		})
 	}
 
-	// Add breadcrumb with cronjob timestamps
+	// Add breadcrumb with pod timestamps
 	scope.AddBreadcrumb(&sentry.Breadcrumb{
 		Message:   fmt.Sprintf("Created pod %s", object.GetName()),
 		Level:     sentry.LevelInfo,
@@ -221,10 +202,10 @@ func jobEnhancer(ctx context.Context, scope *sentry.Scope, object metav1.Object,
 		return errors.New("failed to cast object to Job object")
 	}
 
-	// Add the cronjob to the fingerprint
+	// Add the job to the fingerprint
 	sentryEvent.Fingerprint = append(sentryEvent.Fingerprint, KindJob, jobObj.Name)
 
-	// Add the cronjob to the tag
+	// Add the job to the tag
 	setTagIfNotEmpty(scope, "job_name", object.GetName())
 	jobObj.ManagedFields = []metav1.ManagedFieldsEntry{}
 	metadataJson, err := prettyJson(jobObj.ObjectMeta)
@@ -234,7 +215,7 @@ func jobEnhancer(ctx context.Context, scope *sentry.Scope, object metav1.Object,
 		})
 	}
 
-	// Add breadcrumb with cronjob timestamps
+	// Add breadcrumb with job timestamps
 	scope.AddBreadcrumb(&sentry.Breadcrumb{
 		Message:   fmt.Sprintf("Created job %s", object.GetName()),
 		Level:     sentry.LevelInfo,
@@ -286,10 +267,10 @@ func replicaSetEnhancer(ctx context.Context, scope *sentry.Scope, object metav1.
 		return errors.New("failed to cast object to ReplicaSet object")
 	}
 
-	// Add the cronjob to the fingerprint
+	// Add the replicaset to the fingerprint
 	sentryEvent.Fingerprint = append(sentryEvent.Fingerprint, KindReplicaset, replicasetObj.Name)
 
-	// Add the cronjob to the tag
+	// Add the replicaset to the tag
 	setTagIfNotEmpty(scope, "replicaset_name", object.GetName())
 	replicasetObj.ManagedFields = []metav1.ManagedFieldsEntry{}
 	metadataJson, err := prettyJson(replicasetObj.ObjectMeta)
@@ -299,7 +280,7 @@ func replicaSetEnhancer(ctx context.Context, scope *sentry.Scope, object metav1.
 		})
 	}
 
-	// Add breadcrumb with cronjob timestamps
+	// Add breadcrumb with replicaset timestamps
 	scope.AddBreadcrumb(&sentry.Breadcrumb{
 		Message:   fmt.Sprintf("Created replicaset %s", object.GetName()),
 		Level:     sentry.LevelInfo,
@@ -315,10 +296,10 @@ func deploymentEnhancer(ctx context.Context, scope *sentry.Scope, object metav1.
 	if !ok {
 		return errors.New("failed to cast object to Deployment object")
 	}
-	// Add the cronjob to the fingerprint
+	// Add the deployment to the fingerprint
 	sentryEvent.Fingerprint = append(sentryEvent.Fingerprint, KindDeployment, deploymentObj.Name)
 
-	// Add the cronjob to the tag
+	// Add the deployment to the tag
 	setTagIfNotEmpty(scope, "deployment_name", object.GetName())
 	deploymentObj.ManagedFields = []metav1.ManagedFieldsEntry{}
 	metadataJson, err := prettyJson(deploymentObj.ObjectMeta)
@@ -328,7 +309,7 @@ func deploymentEnhancer(ctx context.Context, scope *sentry.Scope, object metav1.
 		})
 	}
 
-	// Add breadcrumb with cronjob timestamps
+	// Add breadcrumb with deployment timestamps
 	scope.AddBreadcrumb(&sentry.Breadcrumb{
 		Message:   fmt.Sprintf("Created deployment %s", object.GetName()),
 		Level:     sentry.LevelInfo,
@@ -337,3 +318,55 @@ func deploymentEnhancer(ctx context.Context, scope *sentry.Scope, object metav1.
 
 	return nil
 }
+
+// Finds the root owning objects of an object
+// and returns an empty slice if the object has
+// no owning objects
+func findRootOwners(ctx context.Context, kindObjPair *KindObjectPair) ([]KindObjectPair, error) {
+
+	// Use DFS to find the leaves of the owner references graph
+	rootOwners, err := ownerRefDFS(ctx, kindObjPair)
+	if err != nil {
+		return nil, err
+	}
+
+	// If the object has no owner references
+	if rootOwners[0].object.GetUID() == kindObjPair.object.GetUID() {
+		return []KindObjectPair{}, nil
+	}
+
+	return rootOwners, nil
+}
+
+// Performs DFS to find the leaves the owner references graph
+func ownerRefDFS(ctx context.Context, kindObjPair *KindObjectPair) ([]KindObjectPair, error) {
+
+	parents := kindObjPair.object.GetOwnerReferences()
+	// the owners slice to be returned
+	rootOwners := []KindObjectPair{}
+
+	// base case: the object has no parents
+	if len(parents) == 0 {
+		rootOwners = append(rootOwners, *kindObjPair)
+		return rootOwners, nil
+	}
+
+	// recursive case: the object has parents to explore
+	for _, parent := range parents {
+		parentObj, ok := findObject(ctx, parent.Kind, kindObjPair.object.GetNamespace(), parent.Name)
+		if !ok {
+			return nil, errors.New("error attempting to find root owneres")
+		}
+		partialOwners, err := ownerRefDFS(ctx, &KindObjectPair{
+			kind:   parent.Kind,
+			object: parentObj,
+		})
+		if err != nil {
+			return nil, err
+		}
+		if partialOwners != nil {
+			rootOwners = append(rootOwners, partialOwners...)
+		}
+	}
+	return rootOwners, nil
+}