From 92b0649358884faa1a09117759554f73f611a601 Mon Sep 17 00:00:00 2001
From: Aditya Thebe <contact@adityathebe.com>
Date: Wed, 25 Sep 2024 21:48:53 +0545
Subject: [PATCH] feat: show change mapping transormation errors in scraper job
 history

---
 db/update.go                   |  5 ++++-
 scrapers/changes/rules.go      | 14 +++++++++-----
 scrapers/changes/rules_test.go | 21 ++++++++++++++++++++-
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/db/update.go b/db/update.go
index 344ee0e0..3f99331f 100644
--- a/db/update.go
+++ b/db/update.go
@@ -296,7 +296,10 @@ func extractChanges(ctx api.ScrapeContext, result *v1.ScrapeResult, ci *models.C
 
 	logExclusions := ctx.PropertyOn(false, "log.exclusions")
 
-	changes.ProcessRules(result, result.BaseScraper.Transform.Change.Mapping...)
+	if err := changes.ProcessRules(result, result.BaseScraper.Transform.Change.Mapping...); err != nil {
+		ctx.JobHistory().AddError(fmt.Sprintf("error running change mapping transformation: %v", err))
+	}
+
 	for _, changeResult := range result.Changes {
 		if changeResult.Action == v1.Ignore {
 			changeSummary.AddIgnored(changeResult.ChangeType)
diff --git a/scrapers/changes/rules.go b/scrapers/changes/rules.go
index a938a073..96ba98d6 100644
--- a/scrapers/changes/rules.go
+++ b/scrapers/changes/rules.go
@@ -7,6 +7,7 @@ import (
 	"github.com/flanksource/commons/logger"
 	v1 "github.com/flanksource/config-db/api/v1"
 	"github.com/flanksource/gomplate/v3"
+	"github.com/samber/oops"
 	"gopkg.in/yaml.v3"
 )
 
@@ -42,7 +43,7 @@ func (t *changeRule) process(change *v1.ChangeResult) error {
 	}
 
 	if ok, err := evaluateCelExpression(t.Rule, env); err != nil {
-		return fmt.Errorf("failed to evaluate rule %s: %w", t.Rule, err)
+		return fmt.Errorf("failed to evaluate rule: %w", err)
 	} else if !ok {
 		return nil
 	}
@@ -83,9 +84,9 @@ func init() {
 
 // ProcessRules modifies the scraped changes in-place
 // using the change rules.
-func ProcessRules(result *v1.ScrapeResult, rules ...v1.ChangeMapping) {
+func ProcessRules(result *v1.ScrapeResult, rules ...v1.ChangeMapping) error {
 	if len(result.Changes) == 0 {
-		return
+		return nil
 	}
 
 	allRules := Rules
@@ -99,9 +100,10 @@ func ProcessRules(result *v1.ScrapeResult, rules ...v1.ChangeMapping) {
 		})
 	}
 
+	var errors []error
 	for _, rule := range allRules {
 		if match, err := rule.match(result); err != nil {
-			logger.Errorf("Failed to match filter %s: %s", rule.Filter, err)
+			errors = append(errors, oops.Wrapf(err, "failed to match filter"))
 			continue
 		} else if !match {
 			continue
@@ -109,8 +111,10 @@ func ProcessRules(result *v1.ScrapeResult, rules ...v1.ChangeMapping) {
 
 		for i := range result.Changes {
 			if err := rule.process(&result.Changes[i]); err != nil {
-				logger.Errorf("Failed to process rule %s: %v", rule.Rule, err)
+				errors = append(errors, err)
 			}
 		}
 	}
+
+	return oops.Join(errors...)
 }
diff --git a/scrapers/changes/rules_test.go b/scrapers/changes/rules_test.go
index d5660791..5ca43629 100644
--- a/scrapers/changes/rules_test.go
+++ b/scrapers/changes/rules_test.go
@@ -12,7 +12,19 @@ func TestProcessRules(t *testing.T) {
 		name   string
 		input  v1.ScrapeResult
 		expect []v1.ChangeResult
+		rules  []v1.ChangeMapping
+		err    bool
 	}{
+		{
+			name: "Should error out on bad filter",
+			input: v1.ScrapeResult{
+				Changes: []v1.ChangeResult{
+					{ChangeType: "AddTags"},
+				},
+			},
+			rules: []v1.ChangeMapping{{Filter: "bad filter"}},
+			err:   true,
+		},
 		{
 			name: "Test Action: empty ScrapeResult",
 			input: v1.ScrapeResult{
@@ -126,7 +138,14 @@ func TestProcessRules(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			ProcessRules(&tt.input)
+			if err := ProcessRules(&tt.input, tt.rules...); err != nil {
+				if !tt.err {
+					t.Errorf("unexpected Error: %v", err)
+				}
+
+				return
+			}
+
 			if !reflect.DeepEqual(tt.input.Changes, tt.expect) {
 				t.Errorf("ProcessRules() = %v, want %v", tt.input.Changes, tt.expect)
 			}