Skip to content

Commit

Permalink
Fix searchTags and bad dimension name
Browse files Browse the repository at this point in the history
This commit resolves prometheus-community#365.

In cases in which the name of the dimensions for a given metric didn't
match a dimension in our searchTags list, we would pass a default global query
for all metrics under a given namespace into our list, returning metrics
we meant to filter. A test case was added to cover.

In addition, I decorated a debug log entry that was useful in finding
this bug.
  • Loading branch information
femiagbabiaka committed Jul 14, 2022
1 parent b299d6d commit 1d85f70
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 5 deletions.
13 changes: 9 additions & 4 deletions pkg/aws_cloudwatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,19 +307,24 @@ func getFilteredMetricDatas(region string, accountId *string, namespace string,
if len(dimensionNameList) > 0 && !metricDimensionsMatchNames(cwMetric, dimensionNameList) {
continue
}
for _, dimension := range cwMetric.Dimensions {
if dimensionFilterValues, ok := dimensionsFilter[*dimension.Name]; ok {
if d, ok := dimensionFilterValues[*dimension.Value]; !ok {

for _, dimension := range cwMetric.Dimensions { // loop over the returned dimensions for our current metric
if dimensionFilterValues, ok := dimensionsFilter[*dimension.Name]; ok { // if the name of the metric matches the name of a dimension we're filtering against, continue
if d, ok := dimensionFilterValues[*dimension.Value]; !ok { // if the value of our metric doesn't match the value of the dimension we're looking for and we haven't already found our metric in the previous iteration of the loop, skip
if !alreadyFound {
skip = true
}
break
} else {
} else { // if the value of the dimension we're filtering on matches the value attached to our current metric, set alreadyFound to true, and set our metric
alreadyFound = true
r = d
}
} else { // If the dimension name doesn't match the name of a dimension we're filtering on, skip
skip = true
break
}
}

if !skip {
for _, stats := range m.Statistics {
id := fmt.Sprintf("id_%d", rand.Int())
Expand Down
114 changes: 114 additions & 0 deletions pkg/aws_cloudwatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,120 @@ func TestSortyByTimeStamp(t *testing.T) {
require.Equal(t, expectedDataPoints, sortedDataPoints)
}

func Test_getFilteredMetricDatasBadData(t *testing.T) {
type args struct {
region string
accountId *string
namespace string
customTags []Tag
tagsOnMetrics exportedTagsOnMetrics
dimensionRegexps []*string
dimensionNameRequirements []string
resources []*taggedResource
metricsList []*cloudwatch.Metric
m *Metric
}
tests := []struct {
name string
args args
wantGetMetricsData []cloudwatchData
}{
{
"dimensionwrongname",
args{
region: "us-east-1",
accountId: aws.String("123123123123"),
namespace: "ec2",
customTags: nil,
tagsOnMetrics: map[string][]string{
"ec2": {
"Value1",
"Value2",
},
},
dimensionRegexps: SupportedServices.GetService("ec2").DimensionRegexps,
resources: []*taggedResource{
{
ARN: "arn:aws:ec2:us-east-1:123123123123:instance/i-12312312312312312",
Tags: []Tag{
{
Key: "Name",
Value: "some-Node",
},
},
Namespace: "ec2",
Region: "us-east-1",
},
},
metricsList: []*cloudwatch.Metric{
{
MetricName: aws.String("CPUUtilization"),
Dimensions: []*cloudwatch.Dimension{
{
Name: aws.String("BadDimension"),
Value: aws.String("lol"),
},
},
Namespace: aws.String("AWS/EC2"),
},
},
m: &Metric{
Name: "CPUUtilization",
Statistics: []string{
"Average",
},
Period: 60,
Length: 600,
Delay: 120,
NilToZero: aws.Bool(false),
AddCloudwatchTimestamp: aws.Bool(false),
},
},
[]cloudwatchData{
{
AccountId: aws.String("123123123123"),
AddCloudwatchTimestamp: aws.Bool(false),
Dimensions: []*cloudwatch.Dimension{
{
Name: aws.String("InstanceId"),
Value: aws.String("i-12312312312312312"),
},
},
ID: aws.String("arn:aws:ec2:us-east-1:123123123123:instance/i-12312312312312312"),
Metric: aws.String("CPUUtilization"),
Namespace: aws.String("ec2"),
NilToZero: aws.Bool(false),
Period: 60,
Region: aws.String("us-east-1"),
Statistics: []string{
"Average",
},
Tags: []Tag{
{
Key: "Value1",
Value: "",
},
{
Key: "Value2",
Value: "",
},
},
},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := getFilteredMetricDatas(tt.args.region, tt.args.accountId, tt.args.namespace, tt.args.customTags, tt.args.tagsOnMetrics, tt.args.dimensionRegexps, tt.args.resources, tt.args.metricsList, tt.args.dimensionNameRequirements, tt.args.m)
if len(got) > 0 {
t.Errorf("len(getFilteredMetricDatas()) = %v, want 0", len(got))
}
})
}

}

func Test_getFilteredMetricDatas(t *testing.T) {
type args struct {
region string
Expand Down
2 changes: 1 addition & 1 deletion pkg/aws_tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func (iface tagsInterface) get(ctx context.Context, job *Job, region string) ([]
if resource.filterThroughTags(job.SearchTags) {
resources = append(resources, &resource)
} else {
iface.logger.Debug("Skipping resource because search tags do not match")
iface.logger.Debug("Skipping resource because search tags do not match", "arn", resource.ARN)
}
}
return !lastPage
Expand Down

0 comments on commit 1d85f70

Please sign in to comment.