Handle invalid mapping config files (#61)

* When a terminating newline is missing from the metric mapping configuration file, the last definition is not parsed at all. After this change the last 'METRIC_DEFINITION' in the file is equivalent to a newline (either between definitions or as the terminating newline).

* Update mapping, if we are reading the last line of the file. Otherwise we will append the mapping and lose the last label. Since we are updating the mapping in two different places a 'updateMapping' function was introduced.

* Added mapper test for config files without a terminating newline.

* Added another test case where multiple definitions are given without a final terminating newline.

* As suggested by @grobie, we should give an error when a terminating newline is missing. Updated tests accordingly.

mapper.go

@@ -51,6 +51,7 @@ const (
 
 func (m *metricMapper) initFromString(fileContents string) error {
 	lines := strings.Split(fileContents, "\n")
+	numLines := len(lines)
 	state := SEARCHING
 
 	parsedMappings := []metricMapping{}
@@ -76,6 +77,9 @@ func (m *metricMapper) initFromString(fileContents string) error {
 			state = METRIC_DEFINITION
 
 		case METRIC_DEFINITION:
+			if (i == numLines-1) && (line != "") {
+				return fmt.Errorf("Line %d: missing terminating newline", i)
+			}
 			if line == "" {
 				if len(currentMapping.labels) == 0 {
 					return fmt.Errorf("Line %d: metric mapping didn't set any labels", i)
@@ -83,23 +87,14 @@ func (m *metricMapper) initFromString(fileContents string) error {
 				if _, ok := currentMapping.labels["name"]; !ok {
 					return fmt.Errorf("Line %d: metric mapping didn't set a metric name", i)
 				}
-
 				parsedMappings = append(parsedMappings, currentMapping)
-
 				state = SEARCHING
 				currentMapping = metricMapping{labels: prometheus.Labels{}}
 				continue
 			}
-
+			if err := m.updateMapping(line, i, &currentMapping); err != nil {
-			matches := labelLineRE.FindStringSubmatch(line)
+				return err
-			if len(matches) != 3 {
-				return fmt.Errorf("Line %d: expected label mapping line, got: %s", i, line)
 			}
-			label, value := matches[1], matches[2]
-			if label == "name" && !metricNameRE.MatchString(value) {
-				return fmt.Errorf("Line %d: metric name '%s' doesn't match regex '%s'", i, value, metricNameRE)
-			}
-			currentMapping.labels[label] = value
 		default:
 			panic("illegal state")
 		}
@@ -142,3 +137,16 @@ func (m *metricMapper) getMapping(statsdMetric string) (labels prometheus.Labels
 
 	return nil, false
 }
+
+func (m *metricMapper) updateMapping(line string, i int, mapping *metricMapping) error {
+	matches := labelLineRE.FindStringSubmatch(line)
+	if len(matches) != 3 {
+		return fmt.Errorf("Line %d: expected label mapping line, got: %s", i, line)
+	}
+	label, value := matches[1], matches[2]
+	if label == "name" && !metricNameRE.MatchString(value) {
+		return fmt.Errorf("Line %d: metric name '%s' doesn't match regex '%s'", i, value, metricNameRE)
+	}
+	(*mapping).labels[label] = value
+	return nil
+}

mapper_test.go

@@ -134,6 +134,26 @@ func TestMetricMapper(t *testing.T) {
 			`,
 			configBad: true,
 		},
+		// A single mapping config without a terminating newline.
+		{
+			config: `
+				test.*
+				name="name"
+				label="foo"`,
+			configBad: true,
+		},
+		// Multiple mapping configs and no terminating newline.
+		{
+			config: `
+				test.bar
+				name="name_bar"
+				label="foo"
+				
+				test.foo
+				name="name_foo"
+				label="bar"`,
+			configBad: true,
+		},
 	}
 
 	mapper := metricMapper{}