From 575c72b474b18c788b54b264eb06e622d32973f5 Mon Sep 17 00:00:00 2001
From: Raphael Philipe Mendes da Silva <rapphil@gmail.com>
Date: Tue, 27 Sep 2022 19:24:43 -0700
Subject: [PATCH] [exporter/awsemfexporter] Fix regression in dimension roll up
 (#14532)

* [exporter/awsemfexporter]Fix regression in dimension roll up

* Add changelog

* Fix linting errors

* Improve readability

* Improve unit tests
---
 .../awsemfexporter/metric_translator_test.go  | 57 ++++++++++++++-----
 exporter/awsemfexporter/util.go               |  6 +-
 ...xporter-fix-regression-null-dimension.yaml | 11 ++++
 3 files changed, 58 insertions(+), 16 deletions(-)
 create mode 100644 unreleased/awsemfexporter-fix-regression-null-dimension.yaml

diff --git a/exporter/awsemfexporter/metric_translator_test.go b/exporter/awsemfexporter/metric_translator_test.go
index f0780615b86f..c217cfb12680 100644
--- a/exporter/awsemfexporter/metric_translator_test.go
+++ b/exporter/awsemfexporter/metric_translator_test.go
@@ -16,8 +16,8 @@ package awsemfexporter
 
 import (
 	"os"
+	"reflect"
 	"sort"
-	"strings"
 	"testing"
 	"time"
 
@@ -296,17 +296,46 @@ func stringSlicesEqual(expected, actual []string) bool {
 	return true
 }
 
-// hashDimensions hashes dimensions for equality checking.
-func hashDimensions(dims [][]string) []string {
+func min(i, j int) int {
+	if i < j {
+		return i
+	}
+	return j
+}
+
+type dimensionality [][]string
+
+func (d dimensionality) Len() int {
+	return len(d)
+}
+
+func (d dimensionality) Swap(i, j int) {
+	d[i], d[j] = d[j], d[i]
+}
+
+func (d dimensionality) Less(i, j int) bool {
+	dim1 := d[i]
+	dim2 := d[j]
+
+	for k := 0; k < min(len(dim1), len(dim2)); k++ {
+		if dim1[k] != dim2[k] {
+			return dim1[k] < dim2[k]
+		}
+	}
+
+	return len(dim1) < len(dim2)
+}
+
+// normalizes a dimensionality lexicographically so that it can be compared
+func normalizeDimensionality(dims [][]string) [][]string {
 	// Convert to string for easier sorting
-	stringified := make([]string, len(dims))
+	sortedDimensions := make([][]string, len(dims))
 	for i, v := range dims {
 		sort.Strings(v)
-		stringified[i] = strings.Join(v, ",")
+		sortedDimensions[i] = v
 	}
-	// Sort across dimension sets for equality checking
-	sort.Strings(stringified)
-	return stringified
+	sort.Sort(dimensionality(sortedDimensions))
+	return sortedDimensions
 }
 
 // hashMetricSlice hashes a metrics slice for equality checking.
@@ -325,9 +354,9 @@ func hashMetricSlice(metricSlice []map[string]string) []string {
 // (i.e. has same sets of dimensions), regardless of order.
 func assertDimsEqual(t *testing.T, expected, actual [][]string) {
 	assert.Equal(t, len(expected), len(actual))
-	expectedHashedDimensions := hashDimensions(expected)
-	actualHashedDimensions := hashDimensions(actual)
-	assert.Equal(t, expectedHashedDimensions, actualHashedDimensions)
+	expectedDimensions := normalizeDimensionality(expected)
+	actualDimensions := normalizeDimensionality(actual)
+	assert.True(t, reflect.DeepEqual(expectedDimensions, actualDimensions))
 }
 
 // cWMeasurementEqual returns true if CW Measurements are equal.
@@ -351,9 +380,9 @@ func cWMeasurementEqual(expected, actual cWMeasurement) bool {
 	if len(expected.Dimensions) != len(actual.Dimensions) {
 		return false
 	}
-	expectedHashedDimensions := hashDimensions(expected.Dimensions)
-	actualHashedDimensions := hashDimensions(actual.Dimensions)
-	return stringSlicesEqual(expectedHashedDimensions, actualHashedDimensions)
+	expectedDimensions := normalizeDimensionality(expected.Dimensions)
+	actualDimensions := normalizeDimensionality(actual.Dimensions)
+	return reflect.DeepEqual(expectedDimensions, actualDimensions)
 }
 
 // assertCWMeasurementEqual asserts whether CW Measurements are equal.
diff --git a/exporter/awsemfexporter/util.go b/exporter/awsemfexporter/util.go
index fb294ef6de0e..afc2648f095a 100644
--- a/exporter/awsemfexporter/util.go
+++ b/exporter/awsemfexporter/util.go
@@ -131,13 +131,15 @@ func dedupDimensions(dimensions [][]string) (deduped [][]string) {
 // The returned dimensions are sorted in alphabetical order within each dimension set
 func dimensionRollup(dimensionRollupOption string, labels map[string]string) [][]string {
 	var rollupDimensionArray [][]string
-	var dimensionZero []string
+
+	// Empty dimension must be always present in a roll up.
+	dimensionZero := []string{}
 
 	instrLibName, hasOTelKey := labels[oTellibDimensionKey]
 	if hasOTelKey {
 		// If OTel key exists in labels, add it as a zero dimension but remove it
 		// temporarily from labels as it is not an original label
-		dimensionZero = []string{oTellibDimensionKey}
+		dimensionZero = append(dimensionZero, oTellibDimensionKey)
 		delete(labels, oTellibDimensionKey)
 	}
 
diff --git a/unreleased/awsemfexporter-fix-regression-null-dimension.yaml b/unreleased/awsemfexporter-fix-regression-null-dimension.yaml
new file mode 100644
index 000000000000..6a04149e8baf
--- /dev/null
+++ b/unreleased/awsemfexporter-fix-regression-null-dimension.yaml
@@ -0,0 +1,11 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: awsemfexporter
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Fix regression that causes null dimensions to be appended during dimension roll up.
+
+# One or more tracking issues related to the change
+issues: [14532]