From 9575f00ace34d114110e56b99c8fe85b82660a53 Mon Sep 17 00:00:00 2001
From: Julius Volz <julius.volz@gmail.com>
Date: Sun, 24 Dec 2017 12:46:06 +0100
Subject: [PATCH] Treat custom textfile metric timestamps as errors

This is clearer behavior and users will notice and fix their textfiles faster
than if we just output a warning.
---
 .../textfile/client_side_timestamp.out        |  3 +++
 .../client_side_timestamp/metrics.prom        |  2 ++
 .../textfile/two_metric_files/metrics2.prom   |  4 +--
 collector/textfile.go                         | 25 ++++++++++++++-----
 collector/textfile_test.go                    |  4 +++
 5 files changed, 30 insertions(+), 8 deletions(-)
 create mode 100644 collector/fixtures/textfile/client_side_timestamp.out
 create mode 100644 collector/fixtures/textfile/client_side_timestamp/metrics.prom

diff --git a/collector/fixtures/textfile/client_side_timestamp.out b/collector/fixtures/textfile/client_side_timestamp.out
new file mode 100644
index 0000000000..75ad6924b7
--- /dev/null
+++ b/collector/fixtures/textfile/client_side_timestamp.out
@@ -0,0 +1,3 @@
+# HELP node_textfile_scrape_error 1 if there was an error opening or reading a file, 0 otherwise
+# TYPE node_textfile_scrape_error gauge
+node_textfile_scrape_error 1
diff --git a/collector/fixtures/textfile/client_side_timestamp/metrics.prom b/collector/fixtures/textfile/client_side_timestamp/metrics.prom
new file mode 100644
index 0000000000..fe3199bca1
--- /dev/null
+++ b/collector/fixtures/textfile/client_side_timestamp/metrics.prom
@@ -0,0 +1,2 @@
+metric_with_custom_timestamp 1 1441205977284
+normal_metric 2
diff --git a/collector/fixtures/textfile/two_metric_files/metrics2.prom b/collector/fixtures/textfile/two_metric_files/metrics2.prom
index 9d089840b9..81234f9329 100644
--- a/collector/fixtures/textfile/two_metric_files/metrics2.prom
+++ b/collector/fixtures/textfile/two_metric_files/metrics2.prom
@@ -1,2 +1,2 @@
-testmetric2_1{foo="bar"} 30 1441205977284
-testmetric2_2{foo="baz"} 40 1441205977284
+testmetric2_1{foo="bar"} 30
+testmetric2_2{foo="baz"} 40
diff --git a/collector/textfile.go b/collector/textfile.go
index 7922000f7e..1b1b380b4b 100644
--- a/collector/textfile.go
+++ b/collector/textfile.go
@@ -191,9 +191,11 @@ func (c *textFileCollector) Update(ch chan<- prometheus.Metric) error {
 	// Iterate over files and accumulate their metrics.
 	files, err := ioutil.ReadDir(c.path)
 	if err != nil && c.path != "" {
-		log.Errorf("Error reading textfile collector directory %s: %s", c.path, err)
+		log.Errorf("Error reading textfile collector directory %q: %s", c.path, err)
 		error = 1.0
 	}
+
+fileLoop:
 	for _, f := range files {
 		if !strings.HasSuffix(f.Name(), ".prom") {
 			continue
@@ -201,7 +203,7 @@ func (c *textFileCollector) Update(ch chan<- prometheus.Metric) error {
 		path := filepath.Join(c.path, f.Name())
 		file, err := os.Open(path)
 		if err != nil {
-			log.Errorf("Error opening %s: %v", path, err)
+			log.Errorf("Error opening %q: %v", path, err)
 			error = 1.0
 			continue
 		}
@@ -209,18 +211,29 @@ func (c *textFileCollector) Update(ch chan<- prometheus.Metric) error {
 		parsedFamilies, err := parser.TextToMetricFamilies(file)
 		file.Close()
 		if err != nil {
-			log.Errorf("Error parsing %s: %v", path, err)
+			log.Errorf("Error parsing %q: %v", path, err)
 			error = 1.0
 			continue
 		}
-		// Only set this once it has been parsed, so that
-		// a failure does not appear fresh.
-		mtimes[f.Name()] = f.ModTime()
 		for _, mf := range parsedFamilies {
+			for _, m := range mf.Metric {
+				if m.TimestampMs != nil {
+					log.Errorf("Textfile %q contains unsupported client-side timestamps, skipping entire file", path)
+					error = 1.0
+					continue fileLoop
+				}
+			}
 			if mf.Help == nil {
 				help := fmt.Sprintf("Metric read from %s", path)
 				mf.Help = &help
 			}
+		}
+
+		// Only set this once it has been parsed and validated, so that
+		// a failure does not appear fresh.
+		mtimes[f.Name()] = f.ModTime()
+
+		for _, mf := range parsedFamilies {
 			convertMetricFamily(mf, ch)
 		}
 	}
diff --git a/collector/textfile_test.go b/collector/textfile_test.go
index 47e1832523..fb1a700738 100644
--- a/collector/textfile_test.go
+++ b/collector/textfile_test.go
@@ -62,6 +62,10 @@ func TestTextfileCollector(t *testing.T) {
 			path: "fixtures/textfile/nonexistent_path",
 			out:  "fixtures/textfile/nonexistent_path.out",
 		},
+		{
+			path: "fixtures/textfile/client_side_timestamp",
+			out:  "fixtures/textfile/client_side_timestamp.out",
+		},
 		{
 			path: "fixtures/textfile/different_metric_types",
 			out:  "fixtures/textfile/different_metric_types.out",