code refactoring on metric customization component of appsignals

plugins/processors/awsappsignals/customconfiguration/common.go

@@ -4,12 +4,21 @@
 package customconfiguration
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/gobwas/glob"
 	"go.opentelemetry.io/collector/pdata/pcommon"
 )
 
+type AllowListAction string
+
+const (
+	AllowListActionKeep    AllowListAction = "keep"
+	AllowListActionDrop    AllowListAction = "drop"
+	AllowListActionReplace AllowListAction = "replace"
+)
+
 type Selector struct {
 	Dimension string `mapstructure:"dimension"`
 	Match     string `mapstructure:"match"`
@@ -21,10 +30,10 @@ type Replacement struct {
 }
 
 type Rule struct {
-	Selectors    []Selector    `mapstructure:"selectors"`
-	Replacements []Replacement `mapstructure:"replacements,omitempty"`
-	Action       string        `mapstructure:"action"`
-	RuleName     string        `mapstructure:"rule_name,omitempty"`
+	Selectors    []Selector      `mapstructure:"selectors"`
+	Replacements []Replacement   `mapstructure:"replacements,omitempty"`
+	Action       AllowListAction `mapstructure:"action"`
+	RuleName     string          `mapstructure:"rule_name,omitempty"`
 }
 
 type SelectorMatcherItem struct {
@@ -44,6 +53,18 @@ var traceKeyMap = map[string]string{
 	"RemoteOperation": "aws.remote.operation",
 }
 
+func GetAllowListAction(action string) (AllowListAction, error) {
+	switch action {
+	case "drop":
+		return AllowListActionDrop, nil
+	case "keep":
+		return AllowListActionKeep, nil
+	case "replace":
+		return AllowListActionReplace, nil
+	}
+	return "", errors.New("invalid action in rule")
+}
+
 func getExactKey(metricDimensionKey string, isTrace bool) string {
 	if !isTrace {
 		return metricDimensionKey
@@ -56,7 +77,7 @@ func getExactKey(metricDimensionKey string, isTrace bool) string {
 	return traceDimensionKey
 }
 
-func isSelected(attributes pcommon.Map, selectorMatchers []SelectorMatcherItem, isTrace bool) (bool, error) {
+func matchesSelectors(attributes pcommon.Map, selectorMatchers []SelectorMatcherItem, isTrace bool) (bool, error) {
 	for _, item := range selectorMatchers {
 		exactKey := getExactKey(item.Key, isTrace)
 		value, ok := attributes.Get(exactKey)
@@ -82,24 +103,7 @@ func generateSelectorMatchers(selectors []Selector) []SelectorMatcherItem {
 	return selectorMatchers
 }
 
-func generateTestAttributes(service string, operation string, remoteService string, remoteOperation string,
-	isTrace bool) pcommon.Map {
-	attributes := pcommon.NewMap()
-	if isTrace {
-		attributes.PutStr("aws.local.service", service)
-		attributes.PutStr("aws.local.operation", operation)
-		attributes.PutStr("aws.remote.service", remoteService)
-		attributes.PutStr("aws.remote.operation", remoteOperation)
-	} else {
-		attributes.PutStr("Service", service)
-		attributes.PutStr("Operation", operation)
-		attributes.PutStr("RemoteService", remoteService)
-		attributes.PutStr("RemoteOperation", remoteOperation)
-	}
-	return attributes
-}
-
-func generateActionDetails(rules []Rule, action string) []ActionItem {
+func generateActionDetails(rules []Rule, action AllowListAction) []ActionItem {
 	var actionItems []ActionItem
 	for _, rule := range rules {
 		if rule.Action == action {

plugins/processors/awsappsignals/customconfiguration/common_test.go

@@ -0,0 +1,20 @@
+package customconfiguration
+
+import "go.opentelemetry.io/collector/pdata/pcommon"
+
+func generateTestAttributes(service string, operation string, remoteService string, remoteOperation string,
+	isTrace bool) pcommon.Map {
+	attributes := pcommon.NewMap()
+	if isTrace {
+		attributes.PutStr("aws.local.service", service)
+		attributes.PutStr("aws.local.operation", operation)
+		attributes.PutStr("aws.remote.service", remoteService)
+		attributes.PutStr("aws.remote.operation", remoteOperation)
+	} else {
+		attributes.PutStr("Service", service)
+		attributes.PutStr("Operation", operation)
+		attributes.PutStr("RemoteService", remoteService)
+		attributes.PutStr("RemoteOperation", remoteOperation)
+	}
+	return attributes
+}

plugins/processors/awsappsignals/customconfiguration/dropper.go

@@ -9,25 +9,25 @@ type DropActions struct {
 	Actions []ActionItem
 }
 
-func NewCustomDropper(rules []Rule) *DropActions {
+func NewDropper(rules []Rule) *DropActions {
 	return &DropActions{
-		Actions: generateActionDetails(rules, "drop"),
+		Actions: generateActionDetails(rules, AllowListActionDrop),
 	}
 }
 
-func (d *DropActions) ShouldBeDropped(attributes, _ pcommon.Map) (bool, error) {
+func (d *DropActions) ShouldBeDropped(attributes pcommon.Map) (bool, error) {
 	// nothing will be dropped if no rule is defined
 	if d.Actions == nil || len(d.Actions) == 0 {
 		return false, nil
 	}
 	for _, element := range d.Actions {
-		isMatched, err := isSelected(attributes, element.SelectorMatchers, false)
+		isMatched, err := matchesSelectors(attributes, element.SelectorMatchers, false)
 		if isMatched {
-			// The datapoint will be dropped if one of keep rule matched
+			// drop the datapoint as one of drop rules is matched
 			return true, nil
 		}
 		if err != nil {
-			// The datapoint will be kept if error is occurred in match process
+			// keep the datapoint as an error occurred in match process
 			return false, err
 		}
 	}

plugins/processors/awsappsignals/customconfiguration/dropper_test.go

@@ -82,8 +82,7 @@ func TestDropperProcessor(t *testing.T) {
 		},
 	}
 
-	testDropper := NewCustomDropper(config)
-	testMapPlaceHolder := pcommon.NewMap()
+	testDropper := NewDropper(config)
 	isTrace := false
 
 	testCases := []TestCaseForDropper{
@@ -107,17 +106,15 @@ func TestDropperProcessor(t *testing.T) {
 	for i := range testCases {
 		tt := testCases[i]
 		t.Run(tt.name, func(t *testing.T) {
-			result, err := testDropper.ShouldBeDropped(tt.input, testMapPlaceHolder)
+			result, err := testDropper.ShouldBeDropped(tt.input)
 			assert.NoError(t, err)
 			assert.Equal(t, tt.output, result)
 		})
 	}
 }
 
 func TestDropperProcessorWithNilConfig(t *testing.T) {
-
-	testDropper := NewCustomDropper(nil)
-	testMapPlaceHolder := pcommon.NewMap()
+	testDropper := NewDropper(nil)
 	isTrace := false
 
 	testCases := []TestCaseForDropper{
@@ -141,19 +138,17 @@ func TestDropperProcessorWithNilConfig(t *testing.T) {
 	for i := range testCases {
 		tt := testCases[i]
 		t.Run(tt.name, func(t *testing.T) {
-			result, err := testDropper.ShouldBeDropped(tt.input, testMapPlaceHolder)
+			result, err := testDropper.ShouldBeDropped(tt.input)
 			assert.NoError(t, err)
 			assert.Equal(t, tt.output, result)
 		})
 	}
 }
 
 func TestDropperProcessorWithEmptyConfig(t *testing.T) {
+	var config []Rule
 
-	config := []Rule{}
-
-	testDropper := NewCustomDropper(config)
-	testMapPlaceHolder := pcommon.NewMap()
+	testDropper := NewDropper(config)
 	isTrace := false
 
 	testCases := []TestCaseForDropper{
@@ -177,7 +172,7 @@ func TestDropperProcessorWithEmptyConfig(t *testing.T) {
 	for i := range testCases {
 		tt := testCases[i]
 		t.Run(tt.name, func(t *testing.T) {
-			result, err := testDropper.ShouldBeDropped(tt.input, testMapPlaceHolder)
+			result, err := testDropper.ShouldBeDropped(tt.input)
 			assert.NoError(t, err)
 			assert.Equal(t, tt.output, result)
 		})

plugins/processors/awsappsignals/customconfiguration/keeper.go

@@ -9,25 +9,25 @@ type KeepActions struct {
 	Actions []ActionItem
 }
 
-func NewCustomKeeper(rules []Rule) *KeepActions {
+func NewKeeper(rules []Rule) *KeepActions {
 	return &KeepActions{
-		Actions: generateActionDetails(rules, "keep"),
+		Actions: generateActionDetails(rules, AllowListActionKeep),
 	}
 }
 
-func (k *KeepActions) ShouldBeDropped(attributes, _ pcommon.Map) (bool, error) {
+func (k *KeepActions) ShouldBeDropped(attributes pcommon.Map) (bool, error) {
 	// nothing will be dropped if no keep rule is defined
 	if k.Actions == nil || len(k.Actions) == 0 {
 		return false, nil
 	}
 	for _, element := range k.Actions {
-		isMatched, err := isSelected(attributes, element.SelectorMatchers, false)
+		isMatched, err := matchesSelectors(attributes, element.SelectorMatchers, false)
 		if isMatched {
-			// The data point will not be dropped if one of keep rule matched
+			// keep the datapoint as one of the keep rules is matched
 			return false, nil
 		}
 		if err != nil {
-			// The data point will be dropped when error is found
+			// drop the datapoint as an error occurred in match process
 			return true, err
 		}
 	}

plugins/processors/awsappsignals/customconfiguration/keeper_test.go

@@ -69,8 +69,7 @@ func TestKeeperProcessor(t *testing.T) {
 		},
 	}
 
-	testKeeper := NewCustomKeeper(config)
-	testMapPlaceHolder := pcommon.NewMap()
+	testKeeper := NewKeeper(config)
 	isTrace := false
 
 	testCases := []TestCaseForKeeper{
@@ -93,16 +92,15 @@ func TestKeeperProcessor(t *testing.T) {
 	for i := range testCases {
 		tt := testCases[i]
 		t.Run(tt.name, func(t *testing.T) {
-			result, err := testKeeper.ShouldBeDropped(tt.input, testMapPlaceHolder)
+			result, err := testKeeper.ShouldBeDropped(tt.input)
 			assert.NoError(t, err)
 			assert.Equal(t, tt.output, result)
 		})
 	}
 }
 
 func TestKeeperProcessorWithNilConfig(t *testing.T) {
-	testKeeper := NewCustomKeeper(nil)
-	testMapPlaceHolder := pcommon.NewMap()
+	testKeeper := NewKeeper(nil)
 	isTrace := false
 
 	testCases := []TestCaseForKeeper{
@@ -130,7 +128,7 @@ func TestKeeperProcessorWithNilConfig(t *testing.T) {
 	for i := range testCases {
 		tt := testCases[i]
 		t.Run(tt.name, func(t *testing.T) {
-			result, err := testKeeper.ShouldBeDropped(tt.input, testMapPlaceHolder)
+			result, err := testKeeper.ShouldBeDropped(tt.input)
 			assert.NoError(t, err)
 			assert.Equal(t, tt.output, result)
 		})
@@ -141,8 +139,7 @@ func TestKeeperProcessorWithEmptyConfig(t *testing.T) {
 
 	config := []Rule{}
 
-	testKeeper := NewCustomKeeper(config)
-	testMapPlaceHolder := pcommon.NewMap()
+	testKeeper := NewKeeper(config)
 	isTrace := false
 
 	testCases := []TestCaseForKeeper{
@@ -170,7 +167,7 @@ func TestKeeperProcessorWithEmptyConfig(t *testing.T) {
 	for i := range testCases {
 		tt := testCases[i]
 		t.Run(tt.name, func(t *testing.T) {
-			result, err := testKeeper.ShouldBeDropped(tt.input, testMapPlaceHolder)
+			result, err := testKeeper.ShouldBeDropped(tt.input)
 			assert.NoError(t, err)
 			assert.Equal(t, tt.output, result)
 		})

plugins/processors/awsappsignals/customconfiguration/replacer.go

@@ -11,9 +11,9 @@ type ReplaceActions struct {
 	Actions []ActionItem
 }
 
-func NewCustomReplacer(rules []Rule) *ReplaceActions {
+func NewReplacer(rules []Rule) *ReplaceActions {
 	return &ReplaceActions{
-		generateActionDetails(rules, "replace"),
+		generateActionDetails(rules, AllowListActionReplace),
 	}
 }
 
@@ -27,7 +27,7 @@ func (r *ReplaceActions) Process(attributes, _ pcommon.Map, isTrace bool) error
 	finalRules := make(map[string]string)
 	for i := len(actions) - 1; i >= 0; i = i - 1 {
 		element := actions[i]
-		isMatched, _ := isSelected(attributes, element.SelectorMatchers, isTrace)
+		isMatched, _ := matchesSelectors(attributes, element.SelectorMatchers, isTrace)
 		if !isMatched {
 			continue
 		}

plugins/processors/awsappsignals/customconfiguration/replacer_test.go

@@ -71,7 +71,7 @@ func TestReplacerProcess(t *testing.T) {
 		},
 	}
 
-	testReplacer := NewCustomReplacer(config)
+	testReplacer := NewReplacer(config)
 	testMapPlaceHolder := pcommon.NewMap()
 
 	testCases := []TestCaseForReplacer{
@@ -168,7 +168,7 @@ func TestReplacerProcessWithPriority(t *testing.T) {
 		},
 	}
 
-	testReplacer := NewCustomReplacer(config)
+	testReplacer := NewReplacer(config)
 	testMapPlaceHolder := pcommon.NewMap()
 
 	testCases := []TestCaseForReplacer{
@@ -216,7 +216,7 @@ func TestReplacerProcessWithPriority(t *testing.T) {
 
 func TestReplacerProcessWithNilConfig(t *testing.T) {
 
-	testReplacer := NewCustomReplacer(nil)
+	testReplacer := NewReplacer(nil)
 	testMapPlaceHolder := pcommon.NewMap()
 
 	testCases := []TestCaseForReplacer{
@@ -250,7 +250,7 @@ func TestReplacerProcessWithEmptyConfig(t *testing.T) {
 
 	config := []Rule{}
 
-	testReplacer := NewCustomReplacer(config)
+	testReplacer := NewReplacer(config)
 	testMapPlaceHolder := pcommon.NewMap()
 
 	testCases := []TestCaseForReplacer{