Improve filter flag names.

Update netdev and systemd collectors to deprecate poorly chosen flag names.

Old flag names to be removed in 2.0.0.

prometheus#1742

Add log messages for parsed flag values to help discover quoting isuses in
supervisors.

prometheus#1737

Signed-off-by: Ben Kochie <superq@gmail.com>

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## master / unreleased
 
+* [CHANGE] Improve filter flag names.
 * [CHANGE]
 * [FEATURE]
 * [ENHANCEMENT]

collector/filesystem_common.go

@@ -20,6 +20,7 @@ import (
 	"regexp"
 
 	"github.com/go-kit/kit/log"
+	"github.com/go-kit/kit/log/level"
 	"github.com/prometheus/client_golang/prometheus"
 	"gopkg.in/alecthomas/kingpin.v2"
 )
@@ -70,7 +71,9 @@ func init() {
 // NewFilesystemCollector returns a new Collector exposing filesystems stats.
 func NewFilesystemCollector(logger log.Logger) (Collector, error) {
 	subsystem := "filesystem"
+	level.Info(logger).Log("msg", "Parsed flag --collector.filesystem.ignored-mount-points", "flag", *ignoredMountPoints)
 	mountPointPattern := regexp.MustCompile(*ignoredMountPoints)
+	level.Info(logger).Log("msg", "Parsed flag --collector.filesystem.ignored-fs-types", "flag", *ignoredMountPoints)
 	filesystemsTypesPattern := regexp.MustCompile(*ignoredFSTypes)
 
 	sizeDesc := prometheus.NewDesc(

collector/netdev_common.go

@@ -23,21 +23,24 @@ import (
 	"strconv"
 
 	"github.com/go-kit/kit/log"
+	"github.com/go-kit/kit/log/level"
 	"github.com/prometheus/client_golang/prometheus"
 	"gopkg.in/alecthomas/kingpin.v2"
 )
 
 var (
-	netdevIgnoredDevices = kingpin.Flag("collector.netdev.device-blacklist", "Regexp of net devices to blacklist (mutually exclusive to device-whitelist).").String()
-	netdevAcceptDevices  = kingpin.Flag("collector.netdev.device-whitelist", "Regexp of net devices to whitelist (mutually exclusive to device-blacklist).").String()
+	netdevDeviceInclude    = kingpin.Flag("collector.netdev.device-include", "Regexp of net devices to include (mutually exclusive to device-exclude).").String()
+	oldNetdevDeviceInclude = kingpin.Flag("collector.netdev.device-whitelist", "DEPRECATED: Use collector.netdev.device-include").Hidden().String()
+	netdevDeviceExclude    = kingpin.Flag("collector.netdev.device-exclude", "Regexp of net devices to exclude (mutually exclusive to device-include).").String()
+	oldNetdevDeviceExclude = kingpin.Flag("collector.netdev.device-blacklist", "DEPRECATED: Use collector.netdev.device-exclude").Hidden().String()
 )
 
 type netDevCollector struct {
-	subsystem             string
-	ignoredDevicesPattern *regexp.Regexp
-	acceptDevicesPattern  *regexp.Regexp
-	metricDescs           map[string]*prometheus.Desc
-	logger                log.Logger
+	subsystem            string
+	deviceExcludePattern *regexp.Regexp
+	deviceIncludePattern *regexp.Regexp
+	metricDescs          map[string]*prometheus.Desc
+	logger               log.Logger
 }
 
 func init() {
@@ -46,31 +49,51 @@ func init() {
 
 // NewNetDevCollector returns a new Collector exposing network device stats.
 func NewNetDevCollector(logger log.Logger) (Collector, error) {
-	if *netdevIgnoredDevices != "" && *netdevAcceptDevices != "" {
-		return nil, errors.New("device-blacklist & accept-devices are mutually exclusive")
+	if *oldNetdevDeviceInclude != "" {
+		if *netdevDeviceInclude == "" {
+			level.Warn(logger).Log("msg", "--collector.netdev.device-whitelist is DEPRECATED and will be removed in 2.0.0, use --collector.netdev.device-include")
+			*netdevDeviceInclude = *oldNetdevDeviceInclude
+		} else {
+			return nil, errors.New("--collector.netdev.device-whitelist and --collector.netdev.device-include are mutually exclusive")
+		}
+	}
+
+	if *oldNetdevDeviceExclude != "" {
+		if *netdevDeviceExclude == "" {
+			level.Warn(logger).Log("msg", "--collector.netdev.device-blacklist is DEPRECATED and will be removed in 2.0.0, use --collector.netdev.device-exclude")
+			*netdevDeviceExclude = *oldNetdevDeviceExclude
+		} else {
+			return nil, errors.New("--collector.netdev.device-blacklist and --collector.netdev.device-exclude are mutually exclusive")
+		}
+	}
+
+	if *netdevDeviceExclude != "" && *netdevDeviceInclude != "" {
+		return nil, errors.New("device-exclude & device-include are mutually exclusive")
 	}
 
-	var ignorePattern *regexp.Regexp
-	if *netdevIgnoredDevices != "" {
-		ignorePattern = regexp.MustCompile(*netdevIgnoredDevices)
+	var excludePattern *regexp.Regexp
+	if *netdevDeviceExclude != "" {
+		level.Info(logger).Log("msg", "Parsed flag --collector.netdev.device-exclude", "flag", *netdevDeviceExclude)
+		excludePattern = regexp.MustCompile(*netdevDeviceExclude)
 	}
 
-	var acceptPattern *regexp.Regexp
-	if *netdevAcceptDevices != "" {
-		acceptPattern = regexp.MustCompile(*netdevAcceptDevices)
+	var includePattern *regexp.Regexp
+	if *netdevDeviceInclude != "" {
+		level.Info(logger).Log("msg", "Parsed Flag --collector.netdev.device-include", "flag", *netdevDeviceInclude)
+		includePattern = regexp.MustCompile(*netdevDeviceInclude)
 	}
 
 	return &netDevCollector{
-		subsystem:             "network",
-		ignoredDevicesPattern: ignorePattern,
-		acceptDevicesPattern:  acceptPattern,
-		metricDescs:           map[string]*prometheus.Desc{},
-		logger:                logger,
+		subsystem:            "network",
+		deviceExcludePattern: excludePattern,
+		deviceIncludePattern: includePattern,
+		metricDescs:          map[string]*prometheus.Desc{},
+		logger:               logger,
 	}, nil
 }
 
 func (c *netDevCollector) Update(ch chan<- prometheus.Metric) error {
-	netDev, err := getNetDevStats(c.ignoredDevicesPattern, c.acceptDevicesPattern, c.logger)
+	netDev, err := getNetDevStats(c.deviceExcludePattern, c.deviceIncludePattern, c.logger)
 	if err != nil {
 		return fmt.Errorf("couldn't get netstats: %s", err)
 	}

collector/systemd_linux.go

@@ -16,6 +16,7 @@
 package collector
 
 import (
+	"errors"
 	"fmt"
 	"math"
 	"regexp"
@@ -39,8 +40,10 @@ const (
 )
 
 var (
-	unitWhitelist          = kingpin.Flag("collector.systemd.unit-whitelist", "Regexp of systemd units to whitelist. Units must both match whitelist and not match blacklist to be included.").Default(".+").String()
-	unitBlacklist          = kingpin.Flag("collector.systemd.unit-blacklist", "Regexp of systemd units to blacklist. Units must both match whitelist and not match blacklist to be included.").Default(".+\\.(automount|device|mount|scope|slice)").String()
+	unitInclude            = kingpin.Flag("collector.systemd.unit-include", "Regexp of systemd units to include. Units must both match include and not match exclude to be included.").Default(".+").String()
+	oldUnitInclude         = kingpin.Flag("collector.systemd.unit-whitelist", "DEPRECATED: Use --collector.systemd.unit-include").Hidden().String()
+	unitExclude            = kingpin.Flag("collector.systemd.unit-exclude", "Regexp of systemd units to exclude. Units must both match include and not match exclude to be included.").Default(".+\\.(automount|device|mount|scope|slice)").String()
+	oldUnitExclude         = kingpin.Flag("collector.systemd.unit-blacklist", "DEPRECATED: Use collector.systemd.unit-exclude").Hidden().String()
 	systemdPrivate         = kingpin.Flag("collector.systemd.private", "Establish a private, direct connection to systemd without dbus (Strongly discouraged since it requires root. For testing purposes only).").Hidden().Bool()
 	enableTaskMetrics      = kingpin.Flag("collector.systemd.enable-task-metrics", "Enables service unit tasks metrics unit_tasks_current and unit_tasks_max").Bool()
 	enableRestartsMetrics  = kingpin.Flag("collector.systemd.enable-restarts-metrics", "Enables service unit metric service_restart_total").Bool()
@@ -61,8 +64,8 @@ type systemdCollector struct {
 	socketRefusedConnectionsDesc  *prometheus.Desc
 	systemdVersionDesc            *prometheus.Desc
 	systemdVersion                int
-	unitWhitelistPattern          *regexp.Regexp
-	unitBlacklistPattern          *regexp.Regexp
+	unitIncludePattern            *regexp.Regexp
+	unitExcludePattern            *regexp.Regexp
 	logger                        log.Logger
 }
 
@@ -118,8 +121,27 @@ func NewSystemdCollector(logger log.Logger) (Collector, error) {
 	systemdVersionDesc := prometheus.NewDesc(
 		prometheus.BuildFQName(namespace, subsystem, "version"),
 		"Detected systemd version", []string{}, nil)
-	unitWhitelistPattern := regexp.MustCompile(fmt.Sprintf("^(?:%s)$", *unitWhitelist))
-	unitBlacklistPattern := regexp.MustCompile(fmt.Sprintf("^(?:%s)$", *unitBlacklist))
+
+	if *oldUnitExclude != "" {
+		if *unitExclude == "" {
+			level.Warn(logger).Log("msg", "--collector.systemd.unit-blacklist is DEPRECATED and will be removed in 2.0.0, use --collector.systemd.unit-exclude")
+			*unitExclude = *oldUnitExclude
+		} else {
+			return nil, errors.New("--collector.systemd.unit-blacklist and --collector.systemd.unit-exclude are mutually exclusive")
+		}
+	}
+	if *oldUnitInclude != "" {
+		if *unitInclude == "" {
+			level.Warn(logger).Log("msg", "--collector.systemd.unit-whitelist is DEPRECATED and will be removed in 2.0.0, use --collector.systemd.unit-include")
+			*unitInclude = *oldUnitInclude
+		} else {
+			return nil, errors.New("--collector.systemd.unit-whitelist and --collector.systemd.unit-include are mutually exclusive")
+		}
+	}
+	level.Info(logger).Log("msg", "Parsed flag --collector.systemd.unit-include", "flag", *unitInclude)
+	unitIncludePattern := regexp.MustCompile(fmt.Sprintf("^(?:%s)$", *unitInclude))
+	level.Info(logger).Log("msg", "Parsed flag --collector.systemd.unit-exclude", "flag", *unitExclude)
+	unitExcludePattern := regexp.MustCompile(fmt.Sprintf("^(?:%s)$", *unitExclude))
 
 	systemdVersion := getSystemdVersion(logger)
 	if systemdVersion < minSystemdVersionSystemState {
@@ -141,8 +163,8 @@ func NewSystemdCollector(logger log.Logger) (Collector, error) {
 		socketRefusedConnectionsDesc:  socketRefusedConnectionsDesc,
 		systemdVersionDesc:            systemdVersionDesc,
 		systemdVersion:                systemdVersion,
-		unitWhitelistPattern:          unitWhitelistPattern,
-		unitBlacklistPattern:          unitBlacklistPattern,
+		unitIncludePattern:            unitIncludePattern,
+		unitExcludePattern:            unitExcludePattern,
 		logger:                        logger,
 	}, nil
 }
@@ -169,7 +191,7 @@ func (c *systemdCollector) Update(ch chan<- prometheus.Metric) error {
 	level.Debug(c.logger).Log("msg", "collectSummaryMetrics took", "duration_seconds", time.Since(begin).Seconds())
 
 	begin = time.Now()
-	units := filterUnits(allUnits, c.unitWhitelistPattern, c.unitBlacklistPattern, c.logger)
+	units := filterUnits(allUnits, c.unitIncludePattern, c.unitExcludePattern, c.logger)
 	level.Debug(c.logger).Log("msg", "filterUnits took", "duration_seconds", time.Since(begin).Seconds())
 
 	var wg sync.WaitGroup
@@ -443,10 +465,10 @@ func summarizeUnits(units []unit) map[string]float64 {
 	return summarized
 }
 
-func filterUnits(units []unit, whitelistPattern, blacklistPattern *regexp.Regexp, logger log.Logger) []unit {
+func filterUnits(units []unit, includePattern, excludePattern *regexp.Regexp, logger log.Logger) []unit {
 	filtered := make([]unit, 0, len(units))
 	for _, unit := range units {
-		if whitelistPattern.MatchString(unit.Name) && !blacklistPattern.MatchString(unit.Name) && unit.LoadState == "loaded" {
+		if includePattern.MatchString(unit.Name) && !excludePattern.MatchString(unit.Name) && unit.LoadState == "loaded" {
 			level.Debug(logger).Log("msg", "Adding unit", "unit", unit.Name)
 			filtered = append(filtered, unit)
 		} else {

collector/systemd_linux_test.go

@@ -89,11 +89,11 @@ func getUnitListFixtures() [][]unit {
 
 func TestSystemdIgnoreFilter(t *testing.T) {
 	fixtures := getUnitListFixtures()
-	whitelistPattern := regexp.MustCompile("^foo$")
-	blacklistPattern := regexp.MustCompile("^bar$")
-	filtered := filterUnits(fixtures[0], whitelistPattern, blacklistPattern, log.NewNopLogger())
+	includePattern := regexp.MustCompile("^foo$")
+	excludePattern := regexp.MustCompile("^bar$")
+	filtered := filterUnits(fixtures[0], includePattern, excludePattern, log.NewNopLogger())
 	for _, unit := range filtered {
-		if blacklistPattern.MatchString(unit.Name) || !whitelistPattern.MatchString(unit.Name) {
+		if excludePattern.MatchString(unit.Name) || !includePattern.MatchString(unit.Name) {
 			t.Error(unit.Name, "should not be in the filtered list")
 		}
 	}
@@ -106,7 +106,7 @@ func TestSystemdIgnoreFilterDefaultKeepsAll(t *testing.T) {
 	}
 	fixtures := getUnitListFixtures()
 	collector := c.(*systemdCollector)
-	filtered := filterUnits(fixtures[0], collector.unitWhitelistPattern, collector.unitBlacklistPattern, logger)
+	filtered := filterUnits(fixtures[0], collector.unitIncludePattern, collector.unitExcludePattern, logger)
 	// Adjust fixtures by 3 "not-found" units.
 	if len(filtered) != len(fixtures[0])-3 {
 		t.Error("Default filters removed units")