Merge pull request #1743 from prometheus/superq/flags

Improve filter flag names.
This commit is contained in:
Ben Kochie 2020-06-14 10:39:43 +02:00 committed by GitHub
commit 7790f96881
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 86 additions and 37 deletions

View file

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

View file

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

View file

@ -23,19 +23,22 @@ import (
"strconv" "strconv"
"github.com/go-kit/kit/log" "github.com/go-kit/kit/log"
"github.com/go-kit/kit/log/level"
"github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus"
"gopkg.in/alecthomas/kingpin.v2" "gopkg.in/alecthomas/kingpin.v2"
) )
var ( var (
netdevIgnoredDevices = kingpin.Flag("collector.netdev.device-blacklist", "Regexp of net devices to blacklist (mutually exclusive to device-whitelist).").String() netdevDeviceInclude = kingpin.Flag("collector.netdev.device-include", "Regexp of net devices to include (mutually exclusive to device-exclude).").String()
netdevAcceptDevices = kingpin.Flag("collector.netdev.device-whitelist", "Regexp of net devices to whitelist (mutually exclusive to device-blacklist).").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 { type netDevCollector struct {
subsystem string subsystem string
ignoredDevicesPattern *regexp.Regexp deviceExcludePattern *regexp.Regexp
acceptDevicesPattern *regexp.Regexp deviceIncludePattern *regexp.Regexp
metricDescs map[string]*prometheus.Desc metricDescs map[string]*prometheus.Desc
logger log.Logger logger log.Logger
} }
@ -46,31 +49,51 @@ func init() {
// NewNetDevCollector returns a new Collector exposing network device stats. // NewNetDevCollector returns a new Collector exposing network device stats.
func NewNetDevCollector(logger log.Logger) (Collector, error) { func NewNetDevCollector(logger log.Logger) (Collector, error) {
if *netdevIgnoredDevices != "" && *netdevAcceptDevices != "" { if *oldNetdevDeviceInclude != "" {
return nil, errors.New("device-blacklist & accept-devices are mutually exclusive") 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 *oldNetdevDeviceExclude != "" {
if *netdevIgnoredDevices != "" { if *netdevDeviceExclude == "" {
ignorePattern = regexp.MustCompile(*netdevIgnoredDevices) 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 *netdevDeviceExclude != "" && *netdevDeviceInclude != "" {
if *netdevAcceptDevices != "" { return nil, errors.New("device-exclude & device-include are mutually exclusive")
acceptPattern = regexp.MustCompile(*netdevAcceptDevices) }
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{ return &netDevCollector{
subsystem: "network", subsystem: "network",
ignoredDevicesPattern: ignorePattern, deviceExcludePattern: excludePattern,
acceptDevicesPattern: acceptPattern, deviceIncludePattern: includePattern,
metricDescs: map[string]*prometheus.Desc{}, metricDescs: map[string]*prometheus.Desc{},
logger: logger, logger: logger,
}, nil }, nil
} }
func (c *netDevCollector) Update(ch chan<- prometheus.Metric) error { 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 { if err != nil {
return fmt.Errorf("couldn't get netstats: %s", err) return fmt.Errorf("couldn't get netstats: %s", err)
} }

View file

@ -16,6 +16,7 @@
package collector package collector
import ( import (
"errors"
"fmt" "fmt"
"math" "math"
"regexp" "regexp"
@ -39,8 +40,10 @@ const (
) )
var ( 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() 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()
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() 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() 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() 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() 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 socketRefusedConnectionsDesc *prometheus.Desc
systemdVersionDesc *prometheus.Desc systemdVersionDesc *prometheus.Desc
systemdVersion int systemdVersion int
unitWhitelistPattern *regexp.Regexp unitIncludePattern *regexp.Regexp
unitBlacklistPattern *regexp.Regexp unitExcludePattern *regexp.Regexp
logger log.Logger logger log.Logger
} }
@ -118,8 +121,27 @@ func NewSystemdCollector(logger log.Logger) (Collector, error) {
systemdVersionDesc := prometheus.NewDesc( systemdVersionDesc := prometheus.NewDesc(
prometheus.BuildFQName(namespace, subsystem, "version"), prometheus.BuildFQName(namespace, subsystem, "version"),
"Detected systemd version", []string{}, nil) "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) systemdVersion := getSystemdVersion(logger)
if systemdVersion < minSystemdVersionSystemState { if systemdVersion < minSystemdVersionSystemState {
@ -141,8 +163,8 @@ func NewSystemdCollector(logger log.Logger) (Collector, error) {
socketRefusedConnectionsDesc: socketRefusedConnectionsDesc, socketRefusedConnectionsDesc: socketRefusedConnectionsDesc,
systemdVersionDesc: systemdVersionDesc, systemdVersionDesc: systemdVersionDesc,
systemdVersion: systemdVersion, systemdVersion: systemdVersion,
unitWhitelistPattern: unitWhitelistPattern, unitIncludePattern: unitIncludePattern,
unitBlacklistPattern: unitBlacklistPattern, unitExcludePattern: unitExcludePattern,
logger: logger, logger: logger,
}, nil }, 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()) level.Debug(c.logger).Log("msg", "collectSummaryMetrics took", "duration_seconds", time.Since(begin).Seconds())
begin = time.Now() 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()) level.Debug(c.logger).Log("msg", "filterUnits took", "duration_seconds", time.Since(begin).Seconds())
var wg sync.WaitGroup var wg sync.WaitGroup
@ -443,10 +465,10 @@ func summarizeUnits(units []unit) map[string]float64 {
return summarized 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)) filtered := make([]unit, 0, len(units))
for _, unit := range 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) level.Debug(logger).Log("msg", "Adding unit", "unit", unit.Name)
filtered = append(filtered, unit) filtered = append(filtered, unit)
} else { } else {

View file

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