From f5b9ba35d020607f3d685dbf98f622dbb6e7d270 Mon Sep 17 00:00:00 2001 From: Dinifarb Date: Sat, 29 Jul 2023 20:02:47 +0200 Subject: [PATCH 1/6] feat: (#1236) textfile: collect files from multiple path Signed-off-by: Dinifarb --- collector/textfile.go | 179 +++++++++++++++++++++++------------------- 1 file changed, 99 insertions(+), 80 deletions(-) diff --git a/collector/textfile.go b/collector/textfile.go index 38db2450..2d60f713 100644 --- a/collector/textfile.go +++ b/collector/textfile.go @@ -19,7 +19,6 @@ package collector import ( "fmt" "io" - "io/ioutil" "os" "path/filepath" "reflect" @@ -37,11 +36,13 @@ import ( ) const ( - FlagTextFileDirectory = "collector.textfile.directory" + FlagTextFileDirectory = "collector.textfile.directory" + FlagTextFileDirectories = "collector.textfile.directories" ) var ( - textFileDirectory *string + textFileDirectory *string + textFileDirectories *string mtimeDesc = prometheus.NewDesc( prometheus.BuildFQName(Namespace, "textfile", "mtime_seconds"), @@ -54,7 +55,7 @@ var ( type textFileCollector struct { logger log.Logger - path string + directories string // Only set for testing to get predictable output. mtime *float64 } @@ -63,17 +64,27 @@ type textFileCollector struct { func newTextFileCollectorFlags(app *kingpin.Application) { textFileDirectory = app.Flag( FlagTextFileDirectory, - "Directory to read text files with metrics from.", - ).Default(getDefaultPath()).String() + "Directory or Directories to read text files with metrics from.", + ).Default("").String() + textFileDirectories = app.Flag( + FlagTextFileDirectories, + "Directory or Directories to read text files with metrics from.", + ).Default("").String() } // newTextFileCollector returns a new Collector exposing metrics read from files // in the given textfile directory. func newTextFileCollector(logger log.Logger) (Collector, error) { const subsystem = "textfile" + directories := getDefaultPath() + if *textFileDirectory != "" || *textFileDirectories != "" { + directories = *textFileDirectory + "," + *textFileDirectories + directories = strings.Trim(directories, ",") + } + _ = level.Info(logger).Log("msg", fmt.Sprintf("textfile collector directories: %s", directories)) return &textFileCollector{ - logger: log.With(logger, "collector", subsystem), - path: *textFileDirectory, + logger: log.With(logger, "collector", subsystem), + directories: directories, }, nil } @@ -250,89 +261,56 @@ func (cr carriageReturnFilteringReader) Read(p []byte) (int, error) { // Update implements the Collector interface. func (c *textFileCollector) Collect(ctx *ScrapeContext, ch chan<- prometheus.Metric) error { - error := 0.0 - mtimes := map[string]time.Time{} - - // Iterate over files and accumulate their metrics. - files, err := ioutil.ReadDir(c.path) - if err != nil && c.path != "" { - _ = level.Error(c.logger).Log("msg", fmt.Sprintf("Error reading textfile collector directory %q", c.path), "err", err) - error = 1.0 - } - + errorMetric := 0.0 + var mtimes = map[string]time.Time{} // Create empty metricFamily slice here and append parsedFamilies to it inside the loop. // Once loop is complete, raise error if any duplicates are present. // This will ensure that duplicate metrics are correctly detected between multiple .prom files. var metricFamilies = []*dto.MetricFamily{} -fileLoop: - for _, f := range files { - if !strings.HasSuffix(f.Name(), ".prom") { - continue - } - path := filepath.Join(c.path, f.Name()) - _ = level.Debug(c.logger).Log("msg", fmt.Sprintf("Processing file %q", path)) - file, err := os.Open(path) - if err != nil { - _ = level.Error(c.logger).Log("msg", fmt.Sprintf("Error opening %q: %v", path, err)) - error = 1.0 - continue - } - var parser expfmt.TextParser - r, encoding := utfbom.Skip(carriageReturnFilteringReader{r: file}) - if err = checkBOM(encoding); err != nil { - _ = level.Error(c.logger).Log("msg", fmt.Sprintf("Invalid file encoding detected in %s: %s - file must be UTF8", path, err.Error())) - error = 1.0 - continue - } - parsedFamilies, err := parser.TextToMetricFamilies(r) - closeErr := file.Close() - if closeErr != nil { - _ = level.Warn(c.logger).Log("msg", fmt.Sprintf("Error closing file"), "err", err) - } - if err != nil { - _ = level.Error(c.logger).Log("msg", fmt.Sprintf("Error parsing %q: %v", path, err)) - error = 1.0 - continue - } - // Use temporary array to check for duplicates - var families_array []*dto.MetricFamily - - for _, mf := range parsedFamilies { - families_array = append(families_array, mf) - for _, m := range mf.Metric { - if m.TimestampMs != nil { - _ = level.Error(c.logger).Log("msg", fmt.Sprintf("Textfile %q contains unsupported client-side timestamps, skipping entire file", path)) - error = 1.0 - continue fileLoop + // Iterate over files and accumulate their metrics. + for _, directory := range strings.Split(c.directories, ",") { + err := filepath.WalkDir(directory, func(path string, dirEntry os.DirEntry, err error) error { + if err != nil { + _ = level.Error(c.logger).Log("msg", fmt.Sprintf("Error reading directory: %s", path), "err", err) + errorMetric = 1.0 + return nil + } + if !dirEntry.IsDir() && strings.HasSuffix(dirEntry.Name(), ".prom") { + _ = level.Debug(c.logger).Log("msg", fmt.Sprintf("Processing file: %s", path)) + families_array, err := scrapeFile(path, c.logger) + if err != nil { + _ = level.Error(c.logger).Log("msg", fmt.Sprintf("Error screaping file: %q. Skip File.", path), "err", err) + errorMetric = 1.0 + return nil + } + fileInfo, err := os.Stat(path) + if err != nil { + _ = level.Error(c.logger).Log("msg", fmt.Sprintf("Error reading file info: %q. Skip File.", path), "err", err) + errorMetric = 1.0 + return nil + } + if _, hasName := mtimes[fileInfo.Name()]; hasName { + _ = level.Error(c.logger).Log("msg", fmt.Sprintf("Duplicate filename detected: %q. Skip File.", path)) + errorMetric = 1.0 + return nil + } else { + mtimes[fileInfo.Name()] = fileInfo.ModTime() + metricFamilies = append(metricFamilies, families_array...) } } - if mf.Help == nil { - help := fmt.Sprintf("Metric read from %s", path) - mf.Help = &help - } - } - - // If duplicate metrics are detected in a *single* file, skip processing of file metrics - if duplicateMetricEntry(families_array) { - _ = level.Error(c.logger).Log("msg", fmt.Sprintf("Duplicate metrics detected in file %s. Skipping file processing.", f.Name())) - error = 1.0 - continue - } - - // Only set this once it has been parsed and validated, so that - // a failure does not appear fresh. - mtimes[f.Name()] = f.ModTime() - - for _, metricFamily := range parsedFamilies { - metricFamilies = append(metricFamilies, metricFamily) + return nil + }) + if err != nil && directory != "" { + _ = level.Error(c.logger).Log("msg", fmt.Sprintf("Error reading textfile collector directory: %s", c.directories), "err", err) + errorMetric = 1.0 } } // If duplicates are detected across *multiple* files, return error. if duplicateMetricEntry(metricFamilies) { _ = level.Error(c.logger).Log("msg", "Duplicate metrics detected across multiple files") - error = 1.0 + errorMetric = 1.0 } else { for _, mf := range metricFamilies { c.convertMetricFamily(mf, ch) @@ -340,7 +318,6 @@ fileLoop: } c.exportMTimes(mtimes, ch) - // Export if there were errors. ch <- prometheus.MustNewConstMetric( prometheus.NewDesc( @@ -348,11 +325,53 @@ fileLoop: "1 if there was an error opening or reading a file, 0 otherwise", nil, nil, ), - prometheus.GaugeValue, error, + prometheus.GaugeValue, errorMetric, ) return nil } +func scrapeFile(path string, log log.Logger) ([]*dto.MetricFamily, error) { + file, err := os.Open(path) + if err != nil { + return nil, err + } + var parser expfmt.TextParser + r, encoding := utfbom.Skip(carriageReturnFilteringReader{r: file}) + if err = checkBOM(encoding); err != nil { + return nil, err + } + parsedFamilies, err := parser.TextToMetricFamilies(r) + closeErr := file.Close() + if closeErr != nil { + _ = level.Warn(log).Log("msg", fmt.Sprintf("Error closing file %q", path), "err", closeErr) + } + if err != nil { + return nil, err + } + + // Use temporary array to check for duplicates + var families_array []*dto.MetricFamily + + for _, mf := range parsedFamilies { + families_array = append(families_array, mf) + for _, m := range mf.Metric { + if m.TimestampMs != nil { + return nil, fmt.Errorf("textfile contains unsupported client-side timestamps") + } + } + if mf.Help == nil { + help := fmt.Sprintf("Metric read from %s", path) + mf.Help = &help + } + } + + // If duplicate metrics are detected in a *single* file, skip processing of file metrics + if duplicateMetricEntry(families_array) { + return nil, fmt.Errorf("duplicate metrics detected") + } + return families_array, nil +} + func checkBOM(encoding utfbom.Encoding) error { if encoding == utfbom.Unknown || encoding == utfbom.UTF8 { return nil From f9361bb68436bf47be5790466e8a09988d2c7130 Mon Sep 17 00:00:00 2001 From: Dinifarb Date: Sat, 29 Jul 2023 20:03:51 +0200 Subject: [PATCH 2/6] (#1236) textfile: more tests added Signed-off-by: Dinifarb --- collector/textfile_test.go | 74 ++++++++++++++++++- .../duplicate-filename/file.prom | 3 + .../duplicate-filename/sub/file.prom | 3 + .../multiple-dirs/dir1/dir1.prom | 3 + .../multiple-dirs/dir2/dir2.prom | 3 + .../multiple-dirs/dir3/dir3.prom | 3 + .../multiple-dirs/dir3/dir3sub/dir3sub.prom | 3 + 7 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 tools/textfile-test/duplicate-filename/file.prom create mode 100644 tools/textfile-test/duplicate-filename/sub/file.prom create mode 100644 tools/textfile-test/multiple-dirs/dir1/dir1.prom create mode 100644 tools/textfile-test/multiple-dirs/dir2/dir2.prom create mode 100644 tools/textfile-test/multiple-dirs/dir3/dir3.prom create mode 100644 tools/textfile-test/multiple-dirs/dir3/dir3sub/dir3sub.prom diff --git a/collector/textfile_test.go b/collector/textfile_test.go index 0945d9c2..91ab4642 100644 --- a/collector/textfile_test.go +++ b/collector/textfile_test.go @@ -1,19 +1,25 @@ package collector import ( - "io/ioutil" + "fmt" + "io" + "os" "strings" "testing" "github.com/dimchansky/utfbom" + "github.com/go-kit/log" + "github.com/prometheus/client_golang/prometheus" dto "github.com/prometheus/client_model/go" ) +var baseDir = "../tools/textfile-test" + func TestCRFilter(t *testing.T) { sr := strings.NewReader("line 1\r\nline 2") cr := carriageReturnFilteringReader{r: sr} - b, err := ioutil.ReadAll(cr) + b, err := io.ReadAll(cr) if err != nil { t.Error(err) } @@ -153,3 +159,67 @@ func TestDuplicateMetricEntry(t *testing.T) { t.Errorf("Unexpected duplicate found in differentValues") } } + +func TestMultipleDirectories(t *testing.T) { + testDir := baseDir + "/multiple-dirs" + testDirs := fmt.Sprintf("%[1]s/dir1,%[1]s/dir2,%[1]s/dir3", testDir) + collector := &textFileCollector{ + logger: log.NewLogfmtLogger(os.Stdout), + directories: testDirs, + } + scrapeContext, err := PrepareScrapeContext([]string{"textfile_test"}) + if err != nil { + t.Errorf("Unexpected error %s", err) + } + metrics := make(chan prometheus.Metric) + got := "" + go func() { + for { + var metric dto.Metric + val := <-metrics + val.Write(&metric) + got += metric.String() + } + }() + err = collector.Collect(scrapeContext, metrics) + if err != nil { + t.Errorf("Unexpected error %s", err) + } + for _, f := range []string{"dir1", "dir2", "dir3", "dir3sub"} { + if !strings.Contains(got, f) { + t.Errorf("Unexpected output %s: %q", f, got) + } + } +} + +func TestDuplicateFileName(t *testing.T) { + testDir := baseDir + "/duplicate-filename" + collector := &textFileCollector{ + logger: log.NewLogfmtLogger(os.Stdout), + directories: testDir, + } + scrapeContext, err := PrepareScrapeContext([]string{"textfile_test"}) + if err != nil { + t.Errorf("Unexpected error %s", err) + } + metrics := make(chan prometheus.Metric) + got := "" + go func() { + for { + var metric dto.Metric + val := <-metrics + val.Write(&metric) + got += metric.String() + } + }() + err = collector.Collect(scrapeContext, metrics) + if err != nil { + t.Errorf("Unexpected error %s", err) + } + if !strings.Contains(got, "file") { + t.Errorf("Unexpected output %q", got) + } + if strings.Contains(got, "sub_file") { + t.Errorf("Unexpected output %q", got) + } +} diff --git a/tools/textfile-test/duplicate-filename/file.prom b/tools/textfile-test/duplicate-filename/file.prom new file mode 100644 index 00000000..c18aad00 --- /dev/null +++ b/tools/textfile-test/duplicate-filename/file.prom @@ -0,0 +1,3 @@ +# HELP windows_test Some Test +# TYPE windows_test gauge +windows_test{flag="file"} 1 diff --git a/tools/textfile-test/duplicate-filename/sub/file.prom b/tools/textfile-test/duplicate-filename/sub/file.prom new file mode 100644 index 00000000..35839e1d --- /dev/null +++ b/tools/textfile-test/duplicate-filename/sub/file.prom @@ -0,0 +1,3 @@ +# HELP windows_test Some Test +# TYPE windows_test gauge +windows_test{flag="sub_file"} 2 diff --git a/tools/textfile-test/multiple-dirs/dir1/dir1.prom b/tools/textfile-test/multiple-dirs/dir1/dir1.prom new file mode 100644 index 00000000..5ee71d79 --- /dev/null +++ b/tools/textfile-test/multiple-dirs/dir1/dir1.prom @@ -0,0 +1,3 @@ +# HELP windows_test Some Test +# TYPE windows_test gauge +windows_test{flag="dir1"} 1 diff --git a/tools/textfile-test/multiple-dirs/dir2/dir2.prom b/tools/textfile-test/multiple-dirs/dir2/dir2.prom new file mode 100644 index 00000000..c30afdfb --- /dev/null +++ b/tools/textfile-test/multiple-dirs/dir2/dir2.prom @@ -0,0 +1,3 @@ +# HELP windows_test Some Test +# TYPE windows_test gauge +windows_test{flag="dir2"} 2 diff --git a/tools/textfile-test/multiple-dirs/dir3/dir3.prom b/tools/textfile-test/multiple-dirs/dir3/dir3.prom new file mode 100644 index 00000000..ca287f04 --- /dev/null +++ b/tools/textfile-test/multiple-dirs/dir3/dir3.prom @@ -0,0 +1,3 @@ +# HELP windows_test Some Test +# TYPE windows_test gauge +windows_test{flag="dir3"} 3 diff --git a/tools/textfile-test/multiple-dirs/dir3/dir3sub/dir3sub.prom b/tools/textfile-test/multiple-dirs/dir3/dir3sub/dir3sub.prom new file mode 100644 index 00000000..40e972e1 --- /dev/null +++ b/tools/textfile-test/multiple-dirs/dir3/dir3sub/dir3sub.prom @@ -0,0 +1,3 @@ +# HELP windows_test Some Test +# TYPE windows_test gauge +windows_test{flag="dir3sub"} 3 From 7876a465cabc2c0a68dbe4d91dd000d292966f90 Mon Sep 17 00:00:00 2001 From: Dinifarb Date: Sat, 29 Jul 2023 20:53:38 +0200 Subject: [PATCH 3/6] fix: linter issue Signed-off-by: Dinifarb --- collector/textfile.go | 5 ++--- collector/textfile_test.go | 10 ++++++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/collector/textfile.go b/collector/textfile.go index 2d60f713..0fa82e10 100644 --- a/collector/textfile.go +++ b/collector/textfile.go @@ -294,10 +294,9 @@ func (c *textFileCollector) Collect(ctx *ScrapeContext, ch chan<- prometheus.Met _ = level.Error(c.logger).Log("msg", fmt.Sprintf("Duplicate filename detected: %q. Skip File.", path)) errorMetric = 1.0 return nil - } else { - mtimes[fileInfo.Name()] = fileInfo.ModTime() - metricFamilies = append(metricFamilies, families_array...) } + mtimes[fileInfo.Name()] = fileInfo.ModTime() + metricFamilies = append(metricFamilies, families_array...) } return nil }) diff --git a/collector/textfile_test.go b/collector/textfile_test.go index 91ab4642..a398a4da 100644 --- a/collector/textfile_test.go +++ b/collector/textfile_test.go @@ -177,7 +177,10 @@ func TestMultipleDirectories(t *testing.T) { for { var metric dto.Metric val := <-metrics - val.Write(&metric) + err := val.Write(&metric) + if err != nil { + t.Errorf("Unexpected error %s", err) + } got += metric.String() } }() @@ -208,7 +211,10 @@ func TestDuplicateFileName(t *testing.T) { for { var metric dto.Metric val := <-metrics - val.Write(&metric) + err := val.Write(&metric) + if err != nil { + t.Errorf("Unexpected error %s", err) + } got += metric.String() } }() From 0ec4e9d90f323e52ed290a41668c5be17fb9a46f Mon Sep 17 00:00:00 2001 From: Dinifarb Date: Sun, 6 Aug 2023 10:40:47 +0200 Subject: [PATCH 4/6] fix: typo Signed-off-by: Dinifarb --- collector/textfile.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/collector/textfile.go b/collector/textfile.go index 0fa82e10..2f19d018 100644 --- a/collector/textfile.go +++ b/collector/textfile.go @@ -280,7 +280,7 @@ func (c *textFileCollector) Collect(ctx *ScrapeContext, ch chan<- prometheus.Met _ = level.Debug(c.logger).Log("msg", fmt.Sprintf("Processing file: %s", path)) families_array, err := scrapeFile(path, c.logger) if err != nil { - _ = level.Error(c.logger).Log("msg", fmt.Sprintf("Error screaping file: %q. Skip File.", path), "err", err) + _ = level.Error(c.logger).Log("msg", fmt.Sprintf("Error scraping file: %q. Skip File.", path), "err", err) errorMetric = 1.0 return nil } From 9473265ffae89835e0e01a6cc56322d0f2b9068e Mon Sep 17 00:00:00 2001 From: Dinifarb Date: Fri, 11 Aug 2023 21:32:30 +0200 Subject: [PATCH 5/6] readme & text changes Signed-off-by: Dinifarb --- collector/textfile.go | 2 +- docs/collector.textfile.md | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/collector/textfile.go b/collector/textfile.go index 2f19d018..47d17716 100644 --- a/collector/textfile.go +++ b/collector/textfile.go @@ -64,7 +64,7 @@ type textFileCollector struct { func newTextFileCollectorFlags(app *kingpin.Application) { textFileDirectory = app.Flag( FlagTextFileDirectory, - "Directory or Directories to read text files with metrics from.", + "DEPRECATED: Use --collector.textfile.directories", ).Default("").String() textFileDirectories = app.Flag( FlagTextFileDirectories, diff --git a/docs/collector.textfile.md b/docs/collector.textfile.md index 6bb02a20..75d1e5ff 100644 --- a/docs/collector.textfile.md +++ b/docs/collector.textfile.md @@ -10,15 +10,25 @@ Enabled by default? | Yes ## Flags -### `--collector.textfile.directory` +### `--collector.textfile.directory` +:warning: DEPRECATED Use `--collector.textfile.directories` -The directory containing the files to be ingested. Only files with the extension `.prom` are read. The `.prom` file must end with an empty line feed to work properly. +
+ +### `--collector.textfile.directories` +One or multiple directories containing the files to be ingested. + +E.G. `--collector.textfile.directories="C:\MyDir1,C:\MyDir2"` Default value: `C:\Program Files\windows_exporter\textfile_inputs` Required: No -## Metrics +> **Note:** +> - If there are duplicated filenames among the directories, only the first one found will be read. For any other files with the same name, the `windows_textfile_scrape_error` metric will be set to 1 and a error message will be logged. +> - Only files with the extension `.prom` are read. The `.prom` file must end with an empty line feed to work properly. + + Metrics will primarily come from the files on disk. The below listed metrics are collected to give information about the reading of the metrics themselves. @@ -38,7 +48,7 @@ _This collector does not yet have any useful queries added, we would appreciate _This collector does not yet have alerting examples, we would appreciate your help adding them!_ # Example use -This Powershell script, when run in the `collector.textfile.directory` (default `C:\Program Files\windows_exporter\textfile_inputs`), generates a valid `.prom` file that should successfully ingested by windows_exporter. +This Powershell script, when run in the `--collector.textfile.directories` (default `C:\Program Files\windows_exporter\textfile_inputs`), generates a valid `.prom` file that should successfully ingested by windows_exporter. ```Powershell $alpha = 42 From 7530e7b400cef1106cc73d6c7c212595be868dc2 Mon Sep 17 00:00:00 2001 From: Dinifarb Date: Mon, 21 Aug 2023 19:12:12 +0200 Subject: [PATCH 6/6] fix: .hidden() for deprecated directory flag Signed-off-by: Dinifarb --- collector/textfile.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/collector/textfile.go b/collector/textfile.go index 47d17716..6585c729 100644 --- a/collector/textfile.go +++ b/collector/textfile.go @@ -65,7 +65,7 @@ func newTextFileCollectorFlags(app *kingpin.Application) { textFileDirectory = app.Flag( FlagTextFileDirectory, "DEPRECATED: Use --collector.textfile.directories", - ).Default("").String() + ).Default("").Hidden().String() textFileDirectories = app.Flag( FlagTextFileDirectories, "Directory or Directories to read text files with metrics from.",