Improve usability and performance of searchTags (prometheus-community#1270)

Co-authored-by: Cristian Greco <[email protected]>

Author: kgeckhart

docs/configuration.md

@@ -91,7 +91,9 @@ type: <string>
 roles:
   [ - <role_config> ... ]
 
-# List of Key/Value pairs to use for tag filtering (all must match). Value can be a regex.
+# List of Key/Value pairs to use for tag filtering (all must match). 
+# The key is the AWS Tag key and is case-sensitive  
+# The value will be treated as a regex
 searchTags:
   [ - <search_tags_config> ... ]
 

pkg/clients/tagging/v1/client.go

@@ -69,9 +69,27 @@ func (c client) GetResources(ctx context.Context, job model.DiscoveryJob, region
 
 	if len(svc.ResourceFilters) > 0 {
 		shouldHaveDiscoveredResources = true
+
+		var tagFilters []*resourcegroupstaggingapi.TagFilter
+		if len(job.SearchTags) > 0 {
+			for i := range job.SearchTags {
+				// Because everything with the AWS APIs is pointers we need a pointer to the `Key` field from the SearchTag.
+				// We can't take a pointer to any fields from loop variable or the pointer will always be the same and this logic will be broken.
+				st := job.SearchTags[i]
+
+				// AWS's GetResources has a TagFilter option which matches the semantics of our SearchTags where all filters must match
+				// Their value matching implementation is different though so instead of mapping the Key and Value we only map the Keys.
+				// Their API docs say, "If you don't specify a value for a key, the response returns all resources that are tagged with that key, with any or no value."
+				// which makes this a safe way to reduce the amount of data we need to filter out.
+				// https://docs.aws.amazon.com/resourcegroupstagging/latest/APIReference/API_GetResources.html#resourcegrouptagging-GetResources-request-TagFilters
+				tagFilters = append(tagFilters, &resourcegroupstaggingapi.TagFilter{Key: &st.Key})
+			}
+		}
+
 		inputparams := &resourcegroupstaggingapi.GetResourcesInput{
 			ResourceTypeFilters: svc.ResourceFilters,
 			ResourcesPerPage:    aws.Int64(100), // max allowed value according to API docs
+			TagFilters:          tagFilters,
 		}
 		pageNum := 0
 

pkg/clients/tagging/v2/client.go

@@ -12,6 +12,7 @@ import (
 	"github.com/aws/aws-sdk-go-v2/service/databasemigrationservice"
 	"github.com/aws/aws-sdk-go-v2/service/ec2"
 	"github.com/aws/aws-sdk-go-v2/service/resourcegroupstaggingapi"
+	"github.com/aws/aws-sdk-go-v2/service/resourcegroupstaggingapi/types"
 	"github.com/aws/aws-sdk-go-v2/service/shield"
 	"github.com/aws/aws-sdk-go-v2/service/storagegateway"
 
@@ -72,9 +73,25 @@ func (c client) GetResources(ctx context.Context, job model.DiscoveryJob, region
 		for _, filter := range svc.ResourceFilters {
 			filters = append(filters, *filter)
 		}
+		var tagFilters []types.TagFilter
+		if len(job.SearchTags) > 0 {
+			for i := range job.SearchTags {
+				// Because everything with the AWS APIs is pointers we need a pointer to the `Key` field from the SearchTag.
+				// We can't take a pointer to any fields from loop variable or the pointer will always be the same and this logic will be broken.
+				st := job.SearchTags[i]
+
+				// AWS's GetResources has a TagFilter option which matches the semantics of our SearchTags where all filters must match
+				// Their value matching implementation is different though so instead of mapping the Key and Value we only map the Keys.
+				// Their API docs say, "If you don't specify a value for a key, the response returns all resources that are tagged with that key, with any or no value."
+				// which makes this a safe way to reduce the amount of data we need to filter out.
+				// https://docs.aws.amazon.com/resourcegroupstagging/latest/APIReference/API_GetResources.html#resourcegrouptagging-GetResources-request-TagFilters
+				tagFilters = append(tagFilters, types.TagFilter{Key: &st.Key})
+			}
+		}
 		inputparams := &resourcegroupstaggingapi.GetResourcesInput{
 			ResourceTypeFilters: filters,
 			ResourcesPerPage:    aws.Int32(int32(100)), // max allowed value according to API docs
+			TagFilters:          tagFilters,
 		}
 
 		paginator := resourcegroupstaggingapi.NewGetResourcesPaginator(c.taggingAPI, inputparams, func(options *resourcegroupstaggingapi.GetResourcesPaginatorOptions) {

pkg/config/config.go

@@ -6,6 +6,7 @@ import (
 	"os"
 
 	"github.com/aws/aws-sdk-go/aws"
+	"github.com/grafana/regexp"
 	"gopkg.in/yaml.v2"
 
 	"github.com/nerdswords/yet-another-cloudwatch-exporter/pkg/logging"
@@ -208,6 +209,12 @@ func (j *Job) validateDiscoveryJob(jobIdx int) error {
 		}
 	}
 
+	for _, st := range j.SearchTags {
+		if _, err := regexp.Compile(st.Value); err != nil {
+			return fmt.Errorf("Discovery job [%s/%d]: search tag value for %s has invalid regex value %s: %w", j.Type, jobIdx, st.Key, st.Value, err)
+		}
+	}
+
 	return nil
 }
 
@@ -371,7 +378,7 @@ func (c *ScrapeConf) toModelConfig() model.JobsConfig {
 		job.NilToZero = discoveryJob.NilToZero
 		job.AddCloudwatchTimestamp = discoveryJob.AddCloudwatchTimestamp
 		job.Roles = toModelRoles(discoveryJob.Roles)
-		job.SearchTags = toModelTags(discoveryJob.SearchTags)
+		job.SearchTags = toModelSearchTags(discoveryJob.SearchTags)
 		job.CustomTags = toModelTags(discoveryJob.CustomTags)
 		job.Metrics = toModelMetricConfig(discoveryJob.Metrics)
 		job.IncludeContextOnInfoMetrics = discoveryJob.IncludeContextOnInfoMetrics
@@ -435,6 +442,19 @@ func toModelTags(tags []Tag) []model.Tag {
 	return ret
 }
 
+func toModelSearchTags(tags []Tag) []model.SearchTag {
+	ret := make([]model.SearchTag, 0, len(tags))
+	for _, t := range tags {
+		// This should never panic as long as regex validation continues to happen before model mapping
+		r := regexp.MustCompile(t.Value)
+		ret = append(ret, model.SearchTag{
+			Key:   t.Key,
+			Value: r,
+		})
+	}
+	return ret
+}
+
 func toModelRoles(roles []Role) []model.Role {
 	ret := make([]model.Role, 0, len(roles))
 	for _, r := range roles {

pkg/model/model.go

@@ -23,7 +23,7 @@ type DiscoveryJob struct {
 	Regions                     []string
 	Type                        string
 	Roles                       []Role
-	SearchTags                  []Tag
+	SearchTags                  []SearchTag
 	CustomTags                  []Tag
 	DimensionNameRequirements   []string
 	Metrics                     []*MetricConfig
@@ -94,6 +94,11 @@ type Tag struct {
 	Value string
 }
 
+type SearchTag struct {
+	Key   string
+	Value *regexp.Regexp
+}
+
 type Dimension struct {
 	Name  string
 	Value string
@@ -181,25 +186,27 @@ type TaggedResource struct {
 
 // filterThroughTags returns true if all filterTags match
 // with tags of the TaggedResource, returns false otherwise.
-func (r TaggedResource) FilterThroughTags(filterTags []Tag) bool {
+func (r TaggedResource) FilterThroughTags(filterTags []SearchTag) bool {
 	if len(filterTags) == 0 {
 		return true
 	}
 
-	tagMatches := 0
+	tagFilterMatches := 0
 
 	for _, resourceTag := range r.Tags {
 		for _, filterTag := range filterTags {
 			if resourceTag.Key == filterTag.Key {
-				r, _ := regexp.Compile(filterTag.Value)
-				if r.MatchString(resourceTag.Value) {
-					tagMatches++
+				if !filterTag.Value.MatchString(resourceTag.Value) {
+					return false
 				}
+				// A resource needs to match all SearchTags to be returned, so we track the number of tag filter
+				// matches to ensure it matches the number of tag filters at the end
+				tagFilterMatches++
 			}
 		}
 	}
 
-	return tagMatches == len(filterTags)
+	return tagFilterMatches == len(filterTags)
 }
 
 // MetricTags returns a list of tags built from the tags of

pkg/model/model_test.go

@@ -3,14 +3,15 @@ package model
 import (
 	"testing"
 
+	"github.com/grafana/regexp"
 	"github.com/stretchr/testify/require"
 )
 
 func Test_FilterThroughTags(t *testing.T) {
 	testCases := []struct {
 		testName     string
 		resourceTags []Tag
-		filterTags   []Tag
+		filterTags   []SearchTag
 		result       bool
 	}{
 		{
@@ -21,10 +22,10 @@ func Test_FilterThroughTags(t *testing.T) {
 					Value: "v1",
 				},
 			},
-			filterTags: []Tag{
+			filterTags: []SearchTag{
 				{
 					Key:   "k1",
-					Value: "v1",
+					Value: regexp.MustCompile("v1"),
 				},
 			},
 			result: true,
@@ -37,10 +38,10 @@ func Test_FilterThroughTags(t *testing.T) {
 					Value: "v1",
 				},
 			},
-			filterTags: []Tag{
+			filterTags: []SearchTag{
 				{
 					Key:   "k2",
-					Value: "v2",
+					Value: regexp.MustCompile("v2"),
 				},
 			},
 			result: false,
@@ -57,10 +58,10 @@ func Test_FilterThroughTags(t *testing.T) {
 					Value: "v2",
 				},
 			},
-			filterTags: []Tag{
+			filterTags: []SearchTag{
 				{
 					Key:   "k1",
-					Value: "v1",
+					Value: regexp.MustCompile("v1"),
 				},
 			},
 			result: true,
@@ -73,14 +74,14 @@ func Test_FilterThroughTags(t *testing.T) {
 					Value: "v1",
 				},
 			},
-			filterTags: []Tag{
+			filterTags: []SearchTag{
 				{
 					Key:   "k1",
-					Value: "v1",
+					Value: regexp.MustCompile("v1"),
 				},
 				{
 					Key:   "k2",
-					Value: "v2",
+					Value: regexp.MustCompile("v2"),
 				},
 			},
 			result: false,
@@ -93,10 +94,10 @@ func Test_FilterThroughTags(t *testing.T) {
 					Value: "v1",
 				},
 			},
-			filterTags: []Tag{
+			filterTags: []SearchTag{
 				{
 					Key:   "k2",
-					Value: "v1",
+					Value: regexp.MustCompile("v1"),
 				},
 			},
 			result: false,
@@ -109,21 +110,21 @@ func Test_FilterThroughTags(t *testing.T) {
 					Value: "v1",
 				},
 			},
-			filterTags: []Tag{
+			filterTags: []SearchTag{
 				{
 					Key:   "k1",
-					Value: "v2",
+					Value: regexp.MustCompile("v2"),
 				},
 			},
 			result: false,
 		},
 		{
 			testName:     "resource without tags",
 			resourceTags: []Tag{},
-			filterTags: []Tag{
+			filterTags: []SearchTag{
 				{
 					Key:   "k1",
-					Value: "v2",
+					Value: regexp.MustCompile("v2"),
 				},
 			},
 			result: false,
@@ -136,7 +137,7 @@ func Test_FilterThroughTags(t *testing.T) {
 					Value: "v1",
 				},
 			},
-			filterTags: []Tag{},
+			filterTags: []SearchTag{},
 			result:     true,
 		},
 		{
@@ -147,10 +148,10 @@ func Test_FilterThroughTags(t *testing.T) {
 					Value: "v1",
 				},
 			},
-			filterTags: []Tag{
+			filterTags: []SearchTag{
 				{
 					Key:   "k1",
-					Value: "v.*",
+					Value: regexp.MustCompile("v.*"),
 				},
 			},
 			result: true,
@@ -165,7 +166,6 @@ func Test_FilterThroughTags(t *testing.T) {
 				Region:    "us-east-1",
 				Tags:      tc.resourceTags,
 			}
-
 			require.Equal(t, tc.result, res.FilterThroughTags(tc.filterTags))
 		})
 	}