Prometheus fails to recursively load alert rules

[svs1979]

What did you do?
I'd like to load alerts from several subdirectories which are not known in advance. Therefore I created a directory /etc/prometheus/alerts.d where I'll have an amount of subdirectories containing the alert rules.
In Prometheus (2.0.0), I've configured

rule_files:
  - /etc/prometheus/alerts.d/*/*.yml

For testing, I created a structure like that:

/etc/prometheus/alerts.d/common/common.yml
/etc/prometheus/alerts.d/team_1/rules_1.yml
/etc/prometheus/alerts.d/team_2/alerts.yml
/etc/prometheus/alerts.d/team_2/generic.yml
/etc/prometheus/alerts.d/team_3/specific.yml

What did you expect to see?
I'd expect Prometheus to load all of the above rule files

What did you see instead? Under which circumstances?
Prometheus failed to start, giving the error message
level=error ts=2017-12-05T11:49:35.305604677Z caller=main.go:356 msg="Error loading config" err="couldn't load configuration (--config.file=/etc/prometheus/prometheus.yaml): invalid rule file path \"/etc/prometheus/alerts.d/*/*.yml\""

The problem in the sources is that there is an explicit check on the rule_files configuration preventing recursion:

patRulePath   = regexp.MustCompile(`^[^*]*(\*[^/]*)?$`)

(in config/config.go:34). The underlying filepath.Glob() used for resolving the pattern seems to support recursion, though.

Environment

• System information:

  Linux 3.16.0-4-amd64 x86_64

• Prometheus version:

    prometheus, version 2.0.0 (branch: HEAD, revision: 0a74f98628a0463dddc90528220c94de5032d1a0)
    build user:       root@615b82cb36b6
    build date:       20171108-07:11:59
    go version:       go1.9.2

• Prometheus configuration file:

---
global:
  scrape_interval: 60s
  scrape_timeout: 55s
  evaluation_interval: 30s
rule_files:
- /etc/prometheus/alerts.d/*/*.yml
scrape_configs:
- job_name: prometheus
  scrape_interval: 30s
  scrape_timeout: 30s
  static_configs:
  - targets:
    - prometheus.local:9090
    labels:
      alias: prometheus
- job_name: prometheus-nodeexporter
  scrape_interval: 30s
  scrape_timeout: 30s
  static_configs:
  - targets:
    - prometheus.local:9100
    labels:
      alias: nodeexporter

• Logs:

level=info ts=2017-12-05T11:49:32.9629701Z caller=main.go:215 msg="Starting Prometheus" version="(version=2.0.0, branch=HEAD, revision=0a74f98628a0463dddc90528220c94de5032d1a0)"
level=info ts=2017-12-05T11:49:32.963034374Z caller=main.go:216 build_context="(go=go1.9.2, user=root@615b82cb36b6, date=20171108-07:11:59)"
level=info ts=2017-12-05T11:49:32.963062037Z caller=main.go:217 host_details="(Linux 3.16.0-4-amd64 #1 SMP Debian 3.16.43-2+deb8u2 (2017-06-26) x86_64 prometheus (none))"
level=info ts=2017-12-05T11:49:32.965079082Z caller=main.go:314 msg="Starting TSDB"
level=info ts=2017-12-05T11:49:32.965230794Z caller=targetmanager.go:71 component="target manager" msg="Starting target manager..."
level=info ts=2017-12-05T11:49:32.965281383Z caller=web.go:380 component=web msg="Start listening for connections" address=0.0.0.0:9090
level=info ts=2017-12-05T11:49:35.304688526Z caller=main.go:326 msg="TSDB started"
level=info ts=2017-12-05T11:49:35.304762682Z caller=main.go:394 msg="Loading configuration file" filename=/etc/prometheus/prometheus.yaml
level=error ts=2017-12-05T11:49:35.305604677Z caller=main.go:356 msg="Error loading config" err="couldn't load configuration (--config.file=/etc/prometheus/prometheus.yaml): invalid rule file path \"/etc/prometheus/alerts.d/*/*.yml\""

[brian-brazil]

This behaviour is as expected, only the last component of the path can be a glob.

[svs1979]

By "only the last component of the path can be a glob" do you mean the filepath.Glob() or is there something in Prometheus which might get us into trouble here?
A little testing showed that the glob also works with a recursive pattern as the above. Just commenting out the part around pathRulePath.MatchString() in config/config.go (and inserting a log call to make sure all intended alert rules files are loaded in rules/manager.go) did the trick for testing purposes, e.g. something like this:

diff --git a/config/config.go b/config/config.go
index 8a295b89..1078bea6 100644
--- a/config/config.go
+++ b/config/config.go
@@ -356,11 +356,11 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error {
 		c.GlobalConfig = DefaultGlobalConfig
 	}
 
-	for _, rf := range c.RuleFiles {
-		if !patRulePath.MatchString(rf) {
-			return fmt.Errorf("invalid rule file path %q", rf)
-		}
-	}
+	// for _, rf := range c.RuleFiles {
+	// 	if !patRulePath.MatchString(rf) {
+	// 		return fmt.Errorf("invalid rule file path %q", rf)
+	// 	}
+	// }
 	// Do global overrides and validate unique names.
 	jobNames := map[string]struct{}{}
 	for _, scfg := range c.ScrapeConfigs {
diff --git a/rules/manager.go b/rules/manager.go
index 865e80ef..e2e935e1 100644
--- a/rules/manager.go
+++ b/rules/manager.go
@@ -532,6 +532,7 @@ func (m *Manager) loadGroups(interval time.Duration, filenames ...string) (map[s
 	groups := make(map[string]*Group)
 
 	for _, fn := range filenames {
+		level.Info(m.logger).Log("msg", "Adding rule file", "rule_file", fn)
 		rgs, errs := rulefmt.ParseFile(fn)
 		if errs != nil {
 			return nil, errs

[brian-brazil]

There's also globbing in file_sd_configs, which globbing like that won't work for due to inotify.

[svs1979]

Which is handled separately from this or did I miss something?

[juliusv]

Hmm, good point. I think it'd be actually ok for rule files, as we have removed inotify-based config reloading a long time ago. But we still use inotify watches for file_sd files and that only supports the current simple globbing, so not sure if we'd want to support two different kinds of globbing that we need to clarify and document (currently: https://prometheus.io/docs/prometheus/latest/configuration/configuration/#<filename_pattern>)

[brian-brazil]

Yeah, I think consistency is the better choice here.

[gouthamve]

Dupe of #2487?

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.