Merge pull request #106 from prometheus/textfile-collector-fix-and-tests

Fix mtime reporting in textfile collector, add tests.

collector/fixtures/textfile/no_metric_files.out

@@ -0,0 +1,8 @@
+name: "node_textfile_scrape_error"
+help: "1 if there was an error opening or reading a file, 0 otherwise"
+type: GAUGE
+metric: <
+  gauge: <
+    value: 0
+  >
+>

collector/fixtures/textfile/no_metric_files/non_matching_file.txt

@@ -0,0 +1 @@
+This file should be ignored.

collector/fixtures/textfile/nonexistent_path.out

@@ -0,0 +1,8 @@
+name: "node_textfile_scrape_error"
+help: "1 if there was an error opening or reading a file, 0 otherwise"
+type: GAUGE
+metric: <
+  gauge: <
+    value: 1
+  >
+>

collector/fixtures/textfile/two_metric_files.out

@@ -0,0 +1,79 @@
+name: "node_textfile_mtime"
+help: "Unixtime mtime of textfiles successfully read."
+type: GAUGE
+metric: <
+  label: <
+    name: "file"
+    value: "metrics1.prom"
+  >
+  gauge: <
+    value: 1
+  >
+>
+metric: <
+  label: <
+    name: "file"
+    value: "metrics2.prom"
+  >
+  gauge: <
+    value: 2
+  >
+>
+name: "node_textfile_scrape_error"
+help: "1 if there was an error opening or reading a file, 0 otherwise"
+type: GAUGE
+metric: <
+  gauge: <
+    value: 0
+  >
+>
+name: "testmetric1_1"
+help: "Metric read from fixtures/textfile/two_metric_files/metrics1.prom"
+type: UNTYPED
+metric: <
+  label: <
+    name: "foo"
+    value: "bar"
+  >
+  untyped: <
+    value: 10
+  >
+>
+name: "testmetric1_2"
+help: "Metric read from fixtures/textfile/two_metric_files/metrics1.prom"
+type: UNTYPED
+metric: <
+  label: <
+    name: "foo"
+    value: "baz"
+  >
+  untyped: <
+    value: 20
+  >
+>
+name: "testmetric2_1"
+help: "Metric read from fixtures/textfile/two_metric_files/metrics2.prom"
+type: UNTYPED
+metric: <
+  label: <
+    name: "foo"
+    value: "bar"
+  >
+  untyped: <
+    value: 30
+  >
+  timestamp_ms: 1441205977284
+>
+name: "testmetric2_2"
+help: "Metric read from fixtures/textfile/two_metric_files/metrics2.prom"
+type: UNTYPED
+metric: <
+  label: <
+    name: "foo"
+    value: "baz"
+  >
+  untyped: <
+    value: 40
+  >
+  timestamp_ms: 1441205977284
+>

collector/fixtures/textfile/two_metric_files/metrics1.prom

@@ -0,0 +1,2 @@
+testmetric1_1{foo="bar"} 10
+testmetric1_2{foo="baz"} 20

collector/fixtures/textfile/two_metric_files/metrics2.prom

@@ -0,0 +1,2 @@
+testmetric2_1{foo="bar"} 30 1441205977284
+testmetric2_2{foo="baz"} 40 1441205977284

collector/fixtures/textfile/two_metric_files/non_matching_file.txt

@@ -0,0 +1 @@
+This file should be ignored.

collector/textfile.go

@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 	"time"
 
@@ -25,6 +26,7 @@ var (
 )
 
 type textFileCollector struct {
+	path string
 }
 
 func init() {
@@ -34,15 +36,19 @@ func init() {
 // Takes a registers a
 // SetMetricFamilyInjectionHook.
 func NewTextFileCollector() (Collector, error) {
-	if *textFileDirectory == "" {
+	c := &textFileCollector{
+		path: *textFileDirectory,
+	}
+
+	if c.path == "" {
 		// This collector is enabled by default, so do not fail if
 		// the flag is not passed.
 		log.Infof("No directory specified, see --textfile.directory")
 	} else {
-		prometheus.SetMetricFamilyInjectionHook(parseTextFiles)
+		prometheus.SetMetricFamilyInjectionHook(c.parseTextFiles)
 	}
 
-	return &textFileCollector{}, nil
+	return c, nil
 }
 
 // textFile collector works via SetMetricFamilyInjectionHook in parseTextFiles.
@@ -50,26 +56,29 @@ func (c *textFileCollector) Update(ch chan<- prometheus.Metric) (err error) {
 	return nil
 }
 
-func parseTextFiles() []*dto.MetricFamily {
+func (c *textFileCollector) parseTextFiles() []*dto.MetricFamily {
-	var parser text.Parser
 	error := 0.0
 	metricFamilies := make([]*dto.MetricFamily, 0)
 	mtimes := map[string]time.Time{}
 
 	// Iterate over files and accumulate their metrics.
-	files, _ := ioutil.ReadDir(*textFileDirectory)
+	files, err := ioutil.ReadDir(c.path)
+	if err != nil && c.path != "" {
+		log.Errorf("Error reading textfile collector directory %s: %s", c.path, err)
+		error = 1.0
+	}
 	for _, f := range files {
 		if !strings.HasSuffix(f.Name(), ".prom") {
 			continue
 		}
-		path := filepath.Join(*textFileDirectory, f.Name())
+		path := filepath.Join(c.path, f.Name())
 		file, err := os.Open(path)
 		if err != nil {
 			log.Errorf("Error opening %s: %v", path, err)
 			error = 1.0
 			continue
 		}
-		parsedFamilies, err := parser.TextToMetricFamilies(file)
+		parsedFamilies, err := (&text.Parser{}).TextToMetricFamilies(file)
 		if err != nil {
 			log.Errorf("Error parsing %s: %v", path, err)
 			error = 1.0
@@ -95,16 +104,24 @@ func parseTextFiles() []*dto.MetricFamily {
 			Type:   dto.MetricType_GAUGE.Enum(),
 			Metric: []*dto.Metric{},
 		}
-		for name, mtime := range mtimes {
+
+		// Sorting is needed for predictable output comparison in tests.
+		filenames := make([]string, 0, len(mtimes))
+		for filename := range mtimes {
+			filenames = append(filenames, filename)
+		}
+		sort.Strings(filenames)
+
+		for _, filename := range filenames {
 			mtimeMetricFamily.Metric = append(mtimeMetricFamily.Metric,
 				&dto.Metric{
 					Label: []*dto.LabelPair{
 						&dto.LabelPair{
 							Name:  proto.String("file"),
-							Value: &name,
+							Value: proto.String(filename),
 						},
 					},
-					Gauge: &dto.Gauge{Value: proto.Float64(float64(mtime.UnixNano()) / 1e9)},
+					Gauge: &dto.Gauge{Value: proto.Float64(float64(mtimes[filename].UnixNano()) / 1e9)},
 				},
 			)
 		}

collector/textfile_test.go

@@ -0,0 +1,57 @@
+package collector
+
+import (
+	"io/ioutil"
+	"sort"
+	"strings"
+	"testing"
+
+	"github.com/golang/protobuf/proto"
+)
+
+func TestParseTextFiles(t *testing.T) {
+	tests := []struct {
+		path string
+		out  string
+	}{
+		{
+			path: "fixtures/textfile/no_metric_files",
+			out:  "fixtures/textfile/no_metric_files.out",
+		},
+		{
+			path: "fixtures/textfile/two_metric_files",
+			out:  "fixtures/textfile/two_metric_files.out",
+		},
+		{
+			path: "fixtures/textfile/nonexistent_path",
+			out:  "fixtures/textfile/nonexistent_path.out",
+		},
+	}
+
+	for i, test := range tests {
+		c := textFileCollector{
+			path: test.path,
+		}
+
+		mfs := c.parseTextFiles()
+		textMFs := make([]string, 0, len(mfs))
+		for _, mf := range mfs {
+			if mf.GetName() == "node_textfile_mtime" {
+				mf.GetMetric()[0].GetGauge().Value = proto.Float64(1)
+				mf.GetMetric()[1].GetGauge().Value = proto.Float64(2)
+			}
+			textMFs = append(textMFs, proto.MarshalTextString(mf))
+		}
+		sort.Strings(textMFs)
+		got := strings.Join(textMFs, "")
+
+		want, err := ioutil.ReadFile(test.out)
+		if err != nil {
+			t.Fatalf("%d. error reading fixture file %s: %s", i, test.out, err)
+		}
+
+		if string(want) != got {
+			t.Fatalf("%d. want:\n\n%s\n\ngot:\n\n%s", i, string(want), got)
+		}
+	}
+}