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.

https://github.com/prometheus/node_exporter/issues/1742

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

https://github.com/prometheus/node_exporter/issues/1737

Signed-off-by: Ben Kochie <superq@gmail.com>
This commit is contained in:
Ben Kochie 2020-06-12 08:24:29 +02:00
parent 594f417bdf
commit 7e49b68d3a
No known key found for this signature in database
GPG key ID: C646B23C9E3245F1
5 changed files with 86 additions and 37 deletions

View file

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

View file

@ -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(

View file

@ -23,19 +23,22 @@ 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
deviceExcludePattern *regexp.Regexp
deviceIncludePattern *regexp.Regexp
metricDescs map[string]*prometheus.Desc
logger log.Logger
}
@ -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")
}
}
var ignorePattern *regexp.Regexp
if *netdevIgnoredDevices != "" {
ignorePattern = regexp.MustCompile(*netdevIgnoredDevices)
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")
}
}
var acceptPattern *regexp.Regexp
if *netdevAcceptDevices != "" {
acceptPattern = regexp.MustCompile(*netdevAcceptDevices)
if *netdevDeviceExclude != "" && *netdevDeviceInclude != "" {
return nil, errors.New("device-exclude & device-include are mutually exclusive")
}
var excludePattern *regexp.Regexp
if *netdevDeviceExclude != "" {
level.Info(logger).Log("msg", "Parsed flag --collector.netdev.device-exclude", "flag", *netdevDeviceExclude)
excludePattern = regexp.MustCompile(*netdevDeviceExclude)
}
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,
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)
}

View file

@ -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 {

View file

@ -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")