Prevent dimension rollup duplicates.

aws · Sep 20, 2023 · dea7e23 · dea7e23
1 parent 6b25891
commit dea7e23
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 113 deletions.
diff --git a/plugins/outputs/cloudwatch/cloudwatch.go b/plugins/outputs/cloudwatch/cloudwatch.go
@@ -502,13 +502,13 @@ func BuildDimensions(tagMap map[string]string) []*cloudwatch.Dimension {
 	return dimensions
 }
 
-func (c *CloudWatch) ProcessRollup(rawDimension []*cloudwatch.Dimension) [][]*cloudwatch.Dimension {
+func (c *CloudWatch) ProcessRollup(rawDimensions []*cloudwatch.Dimension) [][]*cloudwatch.Dimension {
 	rawDimensionMap := map[string]string{}
-	for _, v := range rawDimension {
+	for _, v := range rawDimensions {
 		rawDimensionMap[*v.Name] = *v.Value
 	}
 	targetDimensionsList := c.config.RollupDimensions
-	fullDimensionsList := [][]*cloudwatch.Dimension{rawDimension}
+	fullDimensionsList := [][]*cloudwatch.Dimension{rawDimensions}
 	for _, targetDimensions := range targetDimensionsList {
 		i := 0
 		extraDimensions := make([]*cloudwatch.Dimension, len(targetDimensions))
@@ -523,7 +523,7 @@ func (c *CloudWatch) ProcessRollup(rawDimension []*cloudwatch.Dimension) [][]*cl
 			}
 			i += 1
 		}
-		if i == len(targetDimensions) && !reflect.DeepEqual(rawDimension, extraDimensions) {
+		if i == len(targetDimensions) && len(targetDimensions) != len(rawDimensions) {
 			fullDimensionsList = append(fullDimensionsList, extraDimensions)
 		}
 	}

diff --git a/plugins/outputs/cloudwatch/cloudwatch_test.go b/plugins/outputs/cloudwatch/cloudwatch_test.go
@@ -100,10 +100,10 @@ func TestProcessRollup(t *testing.T) {
 		publisher.NewNonBlockingFifoQueue(10),
 		10,
 		2*time.Second,
-		cw.WriteToCloudWatch)
-	cw.config.RollupDimensions = [][]string{{"d1", "d2"}, {"d1"}, {}, {"d4"}}
+		cw.WriteToCloudWatch,
+	)
 
-	rawDimension := []*cloudwatch.Dimension{
+	testRawDimensions := []*cloudwatch.Dimension{
 		{
 			Name:  aws.String("d1"),
 			Value: aws.String("v1"),
@@ -118,122 +118,74 @@ func TestProcessRollup(t *testing.T) {
 		},
 	}
 
-	actualDimensionList := cw.ProcessRollup(rawDimension)
-	expectedDimensionList := [][]*cloudwatch.Dimension{
-		{
-			{
-				Name:  aws.String("d1"),
-				Value: aws.String("v1"),
-			},
-			{
-				Name:  aws.String("d2"),
-				Value: aws.String("v2"),
-			},
-			{
-				Name:  aws.String("d3"),
-				Value: aws.String("v3"),
+	testCases := map[string]struct {
+		rollupDimensions [][]string
+		rawDimensions    []*cloudwatch.Dimension
+		want             [][]*cloudwatch.Dimension
+	}{
+		"WithSimpleRollup": {
+			rollupDimensions: [][]string{{"d1", "d2"}, {"d1"}, {}, {"d4"}},
+			rawDimensions:    testRawDimensions,
+			want: [][]*cloudwatch.Dimension{
+				testRawDimensions,
+				{
+					{
+						Name:  aws.String("d1"),
+						Value: aws.String("v1"),
+					},
+					{
+						Name:  aws.String("d2"),
+						Value: aws.String("v2"),
+					},
+				},
+				{
+					{
+						Name:  aws.String("d1"),
+						Value: aws.String("v1"),
+					},
+				},
+				{},
 			},
 		},
-		{
-			{
-				Name:  aws.String("d1"),
-				Value: aws.String("v1"),
-			},
-			{
-				Name:  aws.String("d2"),
-				Value: aws.String("v2"),
-			},
+		"WithNoRollupConfig": {
+			rollupDimensions: [][]string{},
+			rawDimensions:    testRawDimensions,
+			want:             [][]*cloudwatch.Dimension{testRawDimensions},
 		},
-		{
-			{
-				Name:  aws.String("d1"),
-				Value: aws.String("v1"),
-			},
+		"WithNoRawDimensions": {
+			rollupDimensions: [][]string{{"d1", "d2"}, {"d1"}, {}},
+			rawDimensions:    []*cloudwatch.Dimension{},
+			want:             [][]*cloudwatch.Dimension{{}},
 		},
-		{},
-	}
-	assert.EqualValues(t, expectedDimensionList, actualDimensionList, "Unexpected dimension roll up list")
-
-	cw.config.RollupDimensions = [][]string{}
-	rawDimension = []*cloudwatch.Dimension{
-		{
-			Name:  aws.String("d1"),
-			Value: aws.String("v1"),
+		"WithDuplicate/SameOrder": {
+			rollupDimensions: [][]string{{"d1", "d2", "d3"}},
+			rawDimensions:    testRawDimensions,
+			want:             [][]*cloudwatch.Dimension{testRawDimensions},
 		},
-		{
-			Name:  aws.String("d2"),
-			Value: aws.String("v2"),
+		"WithDuplicate/DifferentOrder": {
+			rollupDimensions: [][]string{{"d2", "d1", "d3"}},
+			rawDimensions:    testRawDimensions,
+			want:             [][]*cloudwatch.Dimension{testRawDimensions},
 		},
-		{
-			Name:  aws.String("d3"),
-			Value: aws.String("v3"),
+		"WithSameLength/DifferentNames": {
+			rollupDimensions: [][]string{{"d1", "d3", "d4"}},
+			rawDimensions:    testRawDimensions,
+			want:             [][]*cloudwatch.Dimension{testRawDimensions},
 		},
-	}
-
-	actualDimensionList = cw.ProcessRollup(rawDimension)
-	expectedDimensionList = [][]*cloudwatch.Dimension{
-		{
-			{
-				Name:  aws.String("d1"),
-				Value: aws.String("v1"),
-			},
-			{
-				Name:  aws.String("d2"),
-				Value: aws.String("v2"),
-			},
-			{
-				Name:  aws.String("d3"),
-				Value: aws.String("v3"),
-			},
+		"WithExtraDimensions": {
+			rollupDimensions: [][]string{{"d1", "d2", "d3", "d4"}},
+			rawDimensions:    testRawDimensions,
+			want:             [][]*cloudwatch.Dimension{testRawDimensions},
 		},
 	}
-	assert.EqualValues(t, expectedDimensionList, actualDimensionList, "Unexpected dimension roll up list without rollup setting")
-
-	cw.config.RollupDimensions = [][]string{{"d1", "d2"}, {"d1"}, {}}
-	rawDimension = []*cloudwatch.Dimension{}
-
-	actualDimensionList = cw.ProcessRollup(rawDimension)
-	expectedDimensionList = [][]*cloudwatch.Dimension{
-		{},
-	}
-	assert.EqualValues(t, expectedDimensionList, actualDimensionList, "Unexpected dimension roll up list with no raw dimensions")
-
-	cw.config.RollupDimensions = [][]string{{"d1", "d2", "d3"}}
-	rawDimension = []*cloudwatch.Dimension{
-		{
-			Name:  aws.String("d1"),
-			Value: aws.String("v1"),
-		},
-		{
-			Name:  aws.String("d2"),
-			Value: aws.String("v2"),
-		},
-		{
-			Name:  aws.String("d3"),
-			Value: aws.String("v3"),
-		},
-	}
-
-	actualDimensionList = cw.ProcessRollup(rawDimension)
-	expectedDimensionList = [][]*cloudwatch.Dimension{
-		{
-			{
-				Name:  aws.String("d1"),
-				Value: aws.String("v1"),
-			},
-			{
-				Name:  aws.String("d2"),
-				Value: aws.String("v2"),
-			},
-			{
-				Name:  aws.String("d3"),
-				Value: aws.String("v3"),
-			},
-		},
+	for name, testCase := range testCases {
+		t.Run(name, func(t *testing.T) {
+			cw.config.RollupDimensions = testCase.rollupDimensions
+			got := cw.ProcessRollup(testCase.rawDimensions)
+			assert.EqualValues(t, testCase.want, got, "Unexpected dimension roll up list")
+		})
 	}
-	assert.EqualValues(t, expectedDimensionList, actualDimensionList,
-		"Unexpected dimension roll up list with duplicate roll up")
-	cw.Shutdown(context.Background())
+	assert.NoError(t, cw.Shutdown(context.Background()))
 }
 
 func TestBuildMetricDatumDropUnsupported(t *testing.T) {