Split concerns for buildUnitSuffixes.

By making it return only the appropriate suffixes without modifying them. Character transformation is now done by the caller if necessary.

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

storage/remote/otlptranslator/prometheus/metric_name_builder.go

@@ -129,14 +129,7 @@ func normalizeName(metric pmetric.Metric, namespace string) string {
 	)
 
 	mainUnitSuffix, perUnitSuffix := buildUnitSuffixes(metric.Unit())
-	mainUnitSuffix = cleanUpUnit(mainUnitSuffix)
-	perUnitSuffix = cleanUpUnit(perUnitSuffix)
-	if mainUnitSuffix != "" && !slices.Contains(nameTokens, mainUnitSuffix) {
-		nameTokens = append(nameTokens, mainUnitSuffix)
-	}
-	if perUnitSuffix != "" && !slices.Contains(nameTokens, perUnitSuffix) {
-		nameTokens = append(nameTokens, perUnitSuffix)
-	}
+	nameTokens = addUnitTokens(nameTokens, cleanUpUnit(mainUnitSuffix), cleanUpUnit(perUnitSuffix))
 
 	// Append _total for Counters
 	if metric.Type() == pmetric.MetricTypeSum && metric.Sum().IsMonotonic() {
@@ -168,6 +161,31 @@ func normalizeName(metric pmetric.Metric, namespace string) string {
 	return normalizedName
 }
 
+// addUnitTokens will add the suffixes to the nameTokens if they are not already present.
+// It will also remove trailing underscores from the main suffix to avoid double underscores
+// when joining the tokens.
+func addUnitTokens(nameTokens []string, mainUnitSuffix, perUnitSuffix string) []string {
+	if slices.Contains(nameTokens, mainUnitSuffix) {
+		mainUnitSuffix = ""
+	}
+
+	if slices.Contains(nameTokens, perUnitSuffix) {
+		perUnitSuffix = ""
+	}
+
+	if perUnitSuffix != "" {
+		mainUnitSuffix = strings.TrimSuffix(mainUnitSuffix, "_")
+	}
+
+	if mainUnitSuffix != "" {
+		nameTokens = append(nameTokens, mainUnitSuffix)
+	}
+	if perUnitSuffix != "" {
+		nameTokens = append(nameTokens, perUnitSuffix)
+	}
+	return nameTokens
+}
+
 // cleanUpUnit cleans up unit so it matches model.LabelNameRE.
 func cleanUpUnit(unit string) string {
 	// Multiple consecutive underscores are replaced with a single underscore.
@@ -248,20 +266,23 @@ func BuildMetricName(metric pmetric.Metric, namespace string, addMetricSuffixes
 	return metricName
 }
 
+// buildUnitSuffixes builds the main and per unit suffixes for the specified unit
+// but doesn't do any special character transformation to accommodate Prometheus naming conventions.
+// Removing trailing underscores or appending suffixes is done in the caller.
 func buildUnitSuffixes(unit string) (mainUnitSuffix, perUnitSuffix string) {
 	// Split unit at the '/' if any
 	unitTokens := strings.SplitN(unit, "/", 2)
 
 	if len(unitTokens) > 0 {
 		// Main unit
-		// Append if not blank and doesn't contain '{}'
+		// Update if not blank and doesn't contain '{}'
 		mainUnitOTel := strings.TrimSpace(unitTokens[0])
 		if mainUnitOTel != "" && !strings.ContainsAny(mainUnitOTel, "{}") {
 			mainUnitSuffix = unitMapGetOrDefault(mainUnitOTel)
 		}
 
 		// Per unit
-		// Append if not blank and doesn't contain '{}'
+		// Update if not blank and doesn't contain '{}'
 		if len(unitTokens) > 1 && unitTokens[1] != "" {
 			perUnitOTel := strings.TrimSpace(unitTokens[1])
 			if perUnitOTel != "" && !strings.ContainsAny(perUnitOTel, "{}") {
@@ -269,7 +290,6 @@ func buildUnitSuffixes(unit string) (mainUnitSuffix, perUnitSuffix string) {
 			}
 			if perUnitSuffix != "" {
 				perUnitSuffix = "per_" + perUnitSuffix
-				mainUnitSuffix = strings.TrimSuffix(mainUnitSuffix, "_")
 			}
 		}
 	}

storage/remote/otlptranslator/prometheus/metric_name_builder_test.go

@@ -139,6 +139,49 @@ func TestPerUnitMapGetOrDefault(t *testing.T) {
 	require.Equal(t, "invalid", perUnitMapGetOrDefault("invalid"))
 }
 
+func TestBuildUnitSuffixes(t *testing.T) {
+	tests := []struct {
+		unit         string
+		expectedMain string
+		expectedPer  string
+	}{
+		{"", "", ""},
+		{"s", "seconds", ""},
+		{"By/s", "bytes", "per_second"},
+		{"requests/m", "requests", "per_minute"},
+		{"{invalid}/second", "", "per_second"},
+		{"bytes/{invalid}", "bytes", ""},
+	}
+
+	for _, test := range tests {
+		mainUnitSuffix, perUnitSuffix := buildUnitSuffixes(test.unit)
+		require.Equal(t, test.expectedMain, mainUnitSuffix)
+		require.Equal(t, test.expectedPer, perUnitSuffix)
+	}
+}
+
+func TestAddUnitTokens(t *testing.T) {
+	tests := []struct {
+		nameTokens     []string
+		mainUnitSuffix string
+		perUnitSuffix  string
+		expected       []string
+	}{
+		{[]string{}, "", "", []string{}},
+		{[]string{"token1"}, "main", "", []string{"token1", "main"}},
+		{[]string{"token1"}, "", "per", []string{"token1", "per"}},
+		{[]string{"token1"}, "main", "per", []string{"token1", "main", "per"}},
+		{[]string{"token1", "per"}, "main", "per", []string{"token1", "per", "main"}},
+		{[]string{"token1", "main"}, "main", "per", []string{"token1", "main", "per"}},
+		{[]string{"token1"}, "main_", "per", []string{"token1", "main", "per"}},
+	}
+
+	for _, test := range tests {
+		result := addUnitTokens(test.nameTokens, test.mainUnitSuffix, test.perUnitSuffix)
+		require.Equal(t, test.expected, result)
+	}
+}
+
 func TestRemoveItem(t *testing.T) {
 	require.Equal(t, []string{}, removeItem([]string{}, "test"))
 	require.Equal(t, []string{}, removeItem([]string{}, ""))