update name sanitation to reflect latest definition of what is valid

Author: replay

metric.go

@@ -166,7 +166,7 @@ func (m *MetricDefinition) NameWithTags() string {
 	return m.nameWithTags
 }
 
-func (m *MetricDefinition) NameSanitizedAsTagValue() string {
+func (m *MetricDefinition) NameSanitizedAsTagValue() (string, error) {
 	return SanitizeNameAsTagValue(m.Name)
 }
 
@@ -259,20 +259,27 @@ func MetricDefinitionFromMetricData(d *MetricData) *MetricDefinition {
 // modifies it to ensure it is a valid value that can be
 // used as a tag value. This is important when we index
 // metric names as values of the tag "name"
-func SanitizeNameAsTagValue(name string) string {
-	if !strings.Contains(name, "~") {
-		return name
+func SanitizeNameAsTagValue(name string) (string, error) {
+	if len(name) == 0 {
+		return name, fmt.Errorf("Invalid name of length 0")
 	}
 
-	sanitized := []byte(name)
-	for i := 0; i < len(sanitized); i++ {
-		if sanitized[i] == '~' {
-			sanitized = append(sanitized[:i], sanitized[i+1:]...)
-			i--
+	if name[0] != '~' {
+		return name, nil
+	}
+
+	i := 1
+	for ; i < len(name); i++ {
+		if name[i] != '~' {
+			break
 		}
 	}
 
-	return string(sanitized)
+	if i == len(name) {
+		return "", fmt.Errorf("Invalid name, requiring other characters than ~")
+	}
+
+	return name[i:], nil
 }
 
 // ValidateTags returns whether all tags are in a valid format.

metric_test.go

@@ -137,31 +137,39 @@ func TestNameSanitizedAsTagValue(t *testing.T) {
 	type testCase struct {
 		originalName string
 		expectedName string
+		expectError  bool
 	}
 	cases := []testCase{
 		{
 			originalName: "my~.test.abc",
-			expectedName: "my.test.abc",
+			expectedName: "my~.test.abc",
 		}, {
-			originalName: "a.b.c",
+			originalName: "~a.b.c",
 			expectedName: "a.b.c",
 		}, {
 			originalName: "~~a~~.~~~b~~~.~~~c~~~",
-			expectedName: "a.b.c",
+			expectedName: "a~~.~~~b~~~.~~~c~~~",
 		}, {
-			originalName: "a.b.c~",
-			expectedName: "a.b.c",
+			originalName: "~a~",
+			expectedName: "a~",
 		}, {
-			originalName: "~a.b.c",
-			expectedName: "a.b.c",
+			originalName: "~",
+			expectError:  true,
 		}, {
-			originalName: "~a~",
-			expectedName: "a",
+			originalName: "~~~",
+			expectError:  true,
 		},
 	}
 	for i := range cases {
 		md := MetricDefinition{Name: cases[i].originalName}
-		sanitized := md.NameSanitizedAsTagValue()
+		sanitized, err := md.NameSanitizedAsTagValue()
+		if (err != nil) != cases[i].expectError {
+			if err == nil {
+				t.Fatalf("TC %d: Expected an error, but did not get one", i)
+			} else {
+				t.Fatalf("TC %d: Got an unexpected error: %s", i, err)
+			}
+		}
 		if sanitized != cases[i].expectedName {
 			t.Fatalf("TC %d: Expected sanitized version of %s to be %s, but it was %s", i, md.Name, cases[i].expectedName, sanitized)
 		}