diff --git a/AGENTS.md b/AGENTS.md index fb988a7a..253c1a37 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -148,6 +148,10 @@ exercised a live Windows role. YAML key is the collector name and a `docs/collector..md`; `TestRegistrations` checks that they agree. The configuration file schema in `internal/config` only checks keys: the flags parse and validate the values. + Register include/exclude expression flags with `utils.RegexpFlag`, which + anchors the expression, stores the compiled `*regexp.Regexp` in the config + and names the flag in parse errors; use `utils.IsMatchAll`/`IsMatchNone` to + detect unfiltered defaults. - For textfiles, keep HELP/type metadata consistent across files and preserve unrelated valid series when reporting duplicates or parse failures. Publish producer output by replacing a temporary file in the same directory. File diff --git a/internal/collector/iis/iis.go b/internal/collector/iis/iis.go index a0b3fec9..04f6c0c0 100644 --- a/internal/collector/iis/iis.go +++ b/internal/collector/iis/iis.go @@ -30,6 +30,7 @@ import ( "github.com/alecthomas/kingpin/v2" "github.com/prometheus-community/windows_exporter/internal/mi" "github.com/prometheus-community/windows_exporter/internal/types" + "github.com/prometheus-community/windows_exporter/internal/utils" "github.com/prometheus/client_golang/prometheus" "golang.org/x/sys/windows/registry" ) @@ -99,53 +100,29 @@ func NewWithFlags(app *kingpin.Application) *Collector { config: ConfigDefaults, } - var appExclude, appInclude, siteExclude, siteInclude string - - app.Flag( + utils.RegexpFlag(app, "collector.iis.app-exclude", "Regexp of apps to exclude. App name must both match include and not match exclude to be included.", - ).Default("").StringVar(&appExclude) + "", &c.config.AppExclude, + ) - app.Flag( + utils.RegexpFlag(app, "collector.iis.app-include", "Regexp of apps to include. App name must both match include and not match exclude to be included.", - ).Default(".+").StringVar(&appInclude) + ".+", &c.config.AppInclude, + ) - app.Flag( + utils.RegexpFlag(app, "collector.iis.site-exclude", "Regexp of sites to exclude. Site name must both match include and not match exclude to be included.", - ).Default("").StringVar(&siteExclude) + "", &c.config.SiteExclude, + ) - app.Flag( + utils.RegexpFlag(app, "collector.iis.site-include", "Regexp of sites to include. Site name must both match include and not match exclude to be included.", - ).Default(".+").StringVar(&siteInclude) - - app.Action(func(*kingpin.ParseContext) error { - var err error - - c.config.AppExclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", appExclude)) - if err != nil { - return fmt.Errorf("collector.iis.app-exclude: %w", err) - } - - c.config.AppInclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", appInclude)) - if err != nil { - return fmt.Errorf("collector.iis.app-include: %w", err) - } - - c.config.SiteExclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", siteExclude)) - if err != nil { - return fmt.Errorf("collector.iis.site-exclude: %w", err) - } - - c.config.SiteInclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", siteInclude)) - if err != nil { - return fmt.Errorf("collector.iis.site-include: %w", err) - } - - return nil - }) + ".+", &c.config.SiteInclude, + ) return c } diff --git a/internal/collector/logical_disk/logical_disk.go b/internal/collector/logical_disk/logical_disk.go index 86c1cff8..755d96d1 100644 --- a/internal/collector/logical_disk/logical_disk.go +++ b/internal/collector/logical_disk/logical_disk.go @@ -32,6 +32,7 @@ import ( "github.com/prometheus-community/windows_exporter/internal/mi" "github.com/prometheus-community/windows_exporter/internal/pdh" "github.com/prometheus-community/windows_exporter/internal/types" + "github.com/prometheus-community/windows_exporter/internal/utils" "github.com/prometheus/client_golang/prometheus" "golang.org/x/sys/windows" ) @@ -143,17 +144,19 @@ func NewWithFlags(app *kingpin.Application) *Collector { } c.config.CollectorsEnabled = make([]string, 0) - var collectorsEnabled, volumeExclude, volumeInclude string + var collectorsEnabled string - app.Flag( + utils.RegexpFlag(app, "collector.logical_disk.volume-exclude", "Regexp of volumes to exclude. Volume name must both match include and not match exclude to be included.", - ).Default("").StringVar(&volumeExclude) + "", &c.config.VolumeExclude, + ) - app.Flag( + utils.RegexpFlag(app, "collector.logical_disk.volume-include", "Regexp of volumes to include. Volume name must both match include and not match exclude to be included.", - ).Default(".+").StringVar(&volumeInclude) + ".+", &c.config.VolumeInclude, + ) app.Flag( "collector.logical_disk.enabled", @@ -166,18 +169,6 @@ func NewWithFlags(app *kingpin.Application) *Collector { app.Action(func(*kingpin.ParseContext) error { c.config.CollectorsEnabled = strings.Split(collectorsEnabled, ",") - var err error - - c.config.VolumeExclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", volumeExclude)) - if err != nil { - return fmt.Errorf("collector.logical_disk.volume-exclude: %w", err) - } - - c.config.VolumeInclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", volumeInclude)) - if err != nil { - return fmt.Errorf("collector.logical_disk.volume-include: %w", err) - } - return nil }) diff --git a/internal/collector/logical_disk/logical_disk_test.go b/internal/collector/logical_disk/logical_disk_test.go index 153626b5..ae160796 100644 --- a/internal/collector/logical_disk/logical_disk_test.go +++ b/internal/collector/logical_disk/logical_disk_test.go @@ -22,7 +22,6 @@ import ( "regexp" "testing" - "github.com/alecthomas/kingpin/v2" "github.com/prometheus-community/windows_exporter/internal/collector/logical_disk" "github.com/prometheus-community/windows_exporter/internal/types" "github.com/prometheus-community/windows_exporter/internal/utils/testutils" @@ -31,12 +30,7 @@ import ( ) func BenchmarkCollector(b *testing.B) { - // Whitelist is not set in testing context (kingpin flags not parsed), causing the Collector to skip all disks. - localVolumeInclude := ".+" - - testutils.FuncBenchmarkCollector(b, "logical_disk", logical_disk.NewWithFlags, func(app *kingpin.Application) { - app.GetFlag("collector.logical_disk.volume-include").StringVar(&localVolumeInclude) - }) + testutils.FuncBenchmarkCollector(b, "logical_disk", logical_disk.NewWithFlags) } func TestCollector(t *testing.T) { diff --git a/internal/collector/net/net.go b/internal/collector/net/net.go index eeb8ea53..829f3608 100644 --- a/internal/collector/net/net.go +++ b/internal/collector/net/net.go @@ -34,6 +34,7 @@ import ( "github.com/prometheus-community/windows_exporter/internal/mi" "github.com/prometheus-community/windows_exporter/internal/pdh" "github.com/prometheus-community/windows_exporter/internal/types" + "github.com/prometheus-community/windows_exporter/internal/utils" "github.com/prometheus/client_golang/prometheus" "golang.org/x/sys/windows" ) @@ -151,19 +152,19 @@ func NewWithFlags(app *kingpin.Application) *Collector { } c.config.CollectorsEnabled = make([]string, 0) - var nicExclude, nicInclude string - var collectorsEnabled string - app.Flag( + utils.RegexpFlag(app, "collector.net.nic-exclude", "Regexp of NIC:s to exclude. NIC name must both match include and not match exclude to be included.", - ).Default("").StringVar(&nicExclude) + "", &c.config.NicExclude, + ) - app.Flag( + utils.RegexpFlag(app, "collector.net.nic-include", "Regexp of NIC:s to include. NIC name must both match include and not match exclude to be included.", - ).Default(".+").StringVar(&nicInclude) + ".+", &c.config.NicInclude, + ) app.Flag( "collector.net.enabled", @@ -173,18 +174,6 @@ func NewWithFlags(app *kingpin.Application) *Collector { app.Action(func(*kingpin.ParseContext) error { c.config.CollectorsEnabled = strings.Split(collectorsEnabled, ",") - var err error - - c.config.NicExclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", nicExclude)) - if err != nil { - return fmt.Errorf("collector.net.nic-exclude: %w", err) - } - - c.config.NicInclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", nicInclude)) - if err != nil { - return fmt.Errorf("collector.net.nic-include: %w", err) - } - return nil }) diff --git a/internal/collector/physical_disk/physical_disk.go b/internal/collector/physical_disk/physical_disk.go index 2c139482..fec57aaa 100644 --- a/internal/collector/physical_disk/physical_disk.go +++ b/internal/collector/physical_disk/physical_disk.go @@ -28,6 +28,7 @@ import ( "github.com/prometheus-community/windows_exporter/internal/mi" "github.com/prometheus-community/windows_exporter/internal/pdh" "github.com/prometheus-community/windows_exporter/internal/types" + "github.com/prometheus-community/windows_exporter/internal/utils" "github.com/prometheus/client_golang/prometheus" ) @@ -90,33 +91,17 @@ func NewWithFlags(app *kingpin.Application) *Collector { config: ConfigDefaults, } - var diskExclude, diskInclude string - - app.Flag( + utils.RegexpFlag(app, "collector.physical_disk.disk-exclude", "Regexp of disks to exclude. Disk number must both match include and not match exclude to be included.", - ).Default("").StringVar(&diskExclude) + "", &c.config.DiskExclude, + ) - app.Flag( + utils.RegexpFlag(app, "collector.physical_disk.disk-include", "Regexp of disks to include. Disk number must both match include and not match exclude to be included.", - ).Default(".+").StringVar(&diskInclude) - - app.Action(func(*kingpin.ParseContext) error { - var err error - - c.config.DiskExclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", diskExclude)) - if err != nil { - return fmt.Errorf("collector.physical_disk.disk-exclude: %w", err) - } - - c.config.DiskInclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", diskInclude)) - if err != nil { - return fmt.Errorf("collector.physical_disk.disk-include: %w", err) - } - - return nil - }) + ".+", &c.config.DiskInclude, + ) return c } diff --git a/internal/collector/printer/printer.go b/internal/collector/printer/printer.go index 77c79195..84dae2c2 100644 --- a/internal/collector/printer/printer.go +++ b/internal/collector/printer/printer.go @@ -29,6 +29,7 @@ import ( "github.com/prometheus-community/windows_exporter/internal/metriccache" "github.com/prometheus-community/windows_exporter/internal/mi" "github.com/prometheus-community/windows_exporter/internal/types" + "github.com/prometheus-community/windows_exporter/internal/utils" "github.com/prometheus/client_golang/prometheus" "golang.org/x/sys/windows" ) @@ -112,33 +113,17 @@ func NewWithFlags(app *kingpin.Application) *Collector { config: ConfigDefaults, } - var printerInclude, printerExclude string - - app.Flag( + utils.RegexpFlag(app, "collector.printer.include", "Regular expression to match printers to collect metrics for", - ).Default(".+").StringVar(&printerInclude) + ".+", &c.config.PrinterInclude, + ) - app.Flag( + utils.RegexpFlag(app, "collector.printer.exclude", "Regular expression to match printers to exclude", - ).Default("").StringVar(&printerExclude) - - app.Action(func(*kingpin.ParseContext) error { - var err error - - c.config.PrinterInclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", printerInclude)) - if err != nil { - return fmt.Errorf("collector.printer.include: %w", err) - } - - c.config.PrinterExclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", printerExclude)) - if err != nil { - return fmt.Errorf("collector.printer.exclude: %w", err) - } - - return nil - }) + "", &c.config.PrinterExclude, + ) return c } diff --git a/internal/collector/printer/printer_test.go b/internal/collector/printer/printer_test.go index a863c884..d101e02c 100644 --- a/internal/collector/printer/printer_test.go +++ b/internal/collector/printer/printer_test.go @@ -22,7 +22,6 @@ import ( "testing" "time" - "github.com/alecthomas/kingpin/v2" "github.com/prometheus-community/windows_exporter/internal/collector/printer" "github.com/prometheus-community/windows_exporter/internal/utils/testutils" "github.com/prometheus/client_golang/prometheus" @@ -31,12 +30,7 @@ import ( ) func BenchmarkCollector(b *testing.B) { - // Whitelist is not set in testing context (kingpin flags not parsed), causing the collector to skip all printers. - printersInclude := ".+" - - testutils.FuncBenchmarkCollector(b, "printer", printer.NewWithFlags, func(app *kingpin.Application) { - app.GetFlag("collector.printer.include").StringVar(&printersInclude) - }) + testutils.FuncBenchmarkCollector(b, "printer", printer.NewWithFlags) } func TestCollector(t *testing.T) { diff --git a/internal/collector/process/process.go b/internal/collector/process/process.go index 4a044e96..51b714d9 100644 --- a/internal/collector/process/process.go +++ b/internal/collector/process/process.go @@ -23,7 +23,6 @@ import ( "fmt" "log/slog" "regexp" - "slices" "strconv" "sync" "time" @@ -32,6 +31,7 @@ import ( "github.com/prometheus-community/windows_exporter/internal/labelcache" "github.com/prometheus-community/windows_exporter/internal/mi" "github.com/prometheus-community/windows_exporter/internal/types" + "github.com/prometheus-community/windows_exporter/internal/utils" "github.com/prometheus/client_golang/prometheus" "golang.org/x/sys/windows" ) @@ -129,17 +129,17 @@ func NewWithFlags(app *kingpin.Application) *Collector { config: ConfigDefaults, } - var processExclude, processInclude string - - app.Flag( + utils.RegexpFlag(app, "collector.process.exclude", "Regexp of processes to exclude. Process name must both match include and not match exclude to be included.", - ).Default("").StringVar(&processExclude) + "", &c.config.ProcessExclude, + ) - app.Flag( + utils.RegexpFlag(app, "collector.process.include", "Regexp of processes to include. Process name must both match include and not match exclude to be included.", - ).Default(".+").StringVar(&processInclude) + ".+", &c.config.ProcessInclude, + ) app.Flag( "collector.process.iis", @@ -151,22 +151,6 @@ func NewWithFlags(app *kingpin.Application) *Collector { "If enabled, the full cmdline is exposed to the windows_process_info metrics.", ).Default(strconv.FormatBool(c.config.EnableCMDLine)).BoolVar(&c.config.EnableCMDLine) - app.Action(func(*kingpin.ParseContext) error { - var err error - - c.config.ProcessExclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", processExclude)) - if err != nil { - return fmt.Errorf("collector.process.exclude: %w", err) - } - - c.config.ProcessInclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", processInclude)) - if err != nil { - return fmt.Errorf("collector.process.include: %w", err) - } - - return nil - }) - return c } @@ -195,9 +179,7 @@ func (c *Collector) Build(logger *slog.Logger, miSession *mi.Session) error { c.infoCache = nil c.lookupCache = sync.Map{} - // The flags wrap the expressions in ^(?:...)$, the defaults of [New] are [types.RegExpAny] and [types.RegExpEmpty]. - if slices.Contains([]string{"^(?:.+)$", "^(?:.*)$", types.RegExpAny.String()}, c.config.ProcessInclude.String()) && - slices.Contains([]string{"^(?:)$", types.RegExpEmpty.String()}, c.config.ProcessExclude.String()) { + if utils.IsMatchAll(c.config.ProcessInclude) && utils.IsMatchNone(c.config.ProcessExclude) { logger.Warn("No filters specified for process collector. This will generate a very large number of metrics!") } diff --git a/internal/collector/process/process_internal_test.go b/internal/collector/process/process_internal_test.go index bc32b725..d06102e0 100644 --- a/internal/collector/process/process_internal_test.go +++ b/internal/collector/process/process_internal_test.go @@ -22,10 +22,32 @@ import ( "os/user" "testing" + "github.com/alecthomas/kingpin/v2" "github.com/stretchr/testify/require" "golang.org/x/sys/windows" ) +// The flags store anchored expressions in the configuration of the collector. +func TestNewWithFlagsFilters(t *testing.T) { + t.Parallel() + + app := kingpin.New("test", "test") + c := NewWithFlags(app) + + _, err := app.Parse([]string{"--collector.process.include=foo|bar"}) + require.NoError(t, err) + require.Equal(t, "^(?:foo|bar)$", c.config.ProcessInclude.String()) + require.Equal(t, "^(?:)$", c.config.ProcessExclude.String()) + require.True(t, c.config.ProcessInclude.MatchString("bar")) + require.False(t, c.config.ProcessInclude.MatchString("foobar")) + + app = kingpin.New("test", "test") + NewWithFlags(app) + + _, err = app.Parse([]string{"--collector.process.exclude=("}) + require.ErrorContains(t, err, "collector.process.exclude: error parsing regexp") +} + // The owner label must use the Windows DOMAIN\user form, as os/user reports it. func TestGetProcessOwner(t *testing.T) { t.Parallel() diff --git a/internal/collector/process/process_test.go b/internal/collector/process/process_test.go index 0f6d09d7..3714f5a7 100644 --- a/internal/collector/process/process_test.go +++ b/internal/collector/process/process_test.go @@ -23,19 +23,13 @@ import ( "strconv" "testing" - "github.com/alecthomas/kingpin/v2" "github.com/prometheus-community/windows_exporter/internal/collector/process" "github.com/prometheus-community/windows_exporter/internal/utils/testutils" "github.com/stretchr/testify/require" ) func BenchmarkProcessCollector(b *testing.B) { - // PrinterInclude is not set in testing context (kingpin flags not parsed), causing the collector to skip all processes. - localProcessInclude := ".+" - // No context name required as collector source is WMI - testutils.FuncBenchmarkCollector(b, process.Name, process.NewWithFlags, func(app *kingpin.Application) { - app.GetFlag("collector.process.include").StringVar(&localProcessInclude) - }) + testutils.FuncBenchmarkCollector(b, process.Name, process.NewWithFlags) } func TestCollector(t *testing.T) { diff --git a/internal/collector/scheduled_task/scheduled_task.go b/internal/collector/scheduled_task/scheduled_task.go index d36df524..4f435f07 100644 --- a/internal/collector/scheduled_task/scheduled_task.go +++ b/internal/collector/scheduled_task/scheduled_task.go @@ -33,6 +33,7 @@ import ( "github.com/prometheus-community/windows_exporter/internal/ole" "github.com/prometheus-community/windows_exporter/internal/ole/taskschd" "github.com/prometheus-community/windows_exporter/internal/types" + "github.com/prometheus-community/windows_exporter/internal/utils" "github.com/prometheus/client_golang/prometheus" ) @@ -50,7 +51,7 @@ const defaultTaskExclude = "/Microsoft/.+" //nolint:gochecknoglobals var ConfigDefaults = Config{ - TaskExclude: regexp.MustCompile("^(?:" + defaultTaskExclude + ")$"), + TaskExclude: utils.MustCompileAnchored(defaultTaskExclude), TaskInclude: types.RegExpAny, MissedRuns: false, } @@ -145,18 +146,18 @@ func NewWithFlags(app *kingpin.Application) *Collector { config: ConfigDefaults, } - var taskExclude, taskInclude string - - app.Flag( + utils.RegexpFlag(app, "collector.scheduled_task.exclude", "Regexp of tasks to exclude. Task path must both match include and not match exclude to be included. "+ "The default excludes the tasks of Windows itself.", - ).Default(defaultTaskExclude).StringVar(&taskExclude) + defaultTaskExclude, &c.config.TaskExclude, + ) - app.Flag( + utils.RegexpFlag(app, "collector.scheduled_task.include", "Regexp of tasks to include. Task path must both match include and not match exclude to be included.", - ).Default(".+").StringVar(&taskInclude) + ".+", &c.config.TaskInclude, + ) app.Flag( "collector.scheduled_task.missed-runs", @@ -164,22 +165,6 @@ func NewWithFlags(app *kingpin.Application) *Collector { "Reading the missed runs of a task costs the Task Scheduler service two requests per scrape.", ).Default(strconv.FormatBool(ConfigDefaults.MissedRuns)).BoolVar(&c.config.MissedRuns) - app.Action(func(*kingpin.ParseContext) error { - var err error - - c.config.TaskExclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", taskExclude)) - if err != nil { - return fmt.Errorf("collector.scheduled_task.exclude: %w", err) - } - - c.config.TaskInclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", taskInclude)) - if err != nil { - return fmt.Errorf("collector.scheduled_task.include: %w", err) - } - - return nil - }) - return c } diff --git a/internal/collector/service/service.go b/internal/collector/service/service.go index 6e4875f8..4045f594 100644 --- a/internal/collector/service/service.go +++ b/internal/collector/service/service.go @@ -35,6 +35,7 @@ import ( "github.com/prometheus-community/windows_exporter/internal/metriccache" "github.com/prometheus-community/windows_exporter/internal/mi" "github.com/prometheus-community/windows_exporter/internal/types" + "github.com/prometheus-community/windows_exporter/internal/utils" "github.com/prometheus-community/windows_exporter/internal/utils/recovery" "github.com/prometheus/client_golang/prometheus" "golang.org/x/sys/windows" @@ -143,17 +144,19 @@ func NewWithFlags(app *kingpin.Application) *Collector { } c.config.ServiceStartModeInclude = make([]string, 0) - var serviceExclude, serviceInclude, serviceStartModeInclude string + var serviceStartModeInclude string - app.Flag( + utils.RegexpFlag(app, "collector.service.exclude", "Regexp of service to exclude. Service name (not the display name!) must both match include and not match exclude to be included.", - ).Default("").StringVar(&serviceExclude) + "", &c.config.ServiceExclude, + ) - app.Flag( + utils.RegexpFlag(app, "collector.service.include", "Regexp of service to include. Service name (not the display name!) must both match include and not match exclude to be included.", - ).Default(".+").StringVar(&serviceInclude) + ".+", &c.config.ServiceInclude, + ) app.Flag( "collector.service.start-mode-include", @@ -161,18 +164,6 @@ func NewWithFlags(app *kingpin.Application) *Collector { ).Default(strings.Join(ConfigDefaults.ServiceStartModeInclude, ",")).StringVar(&serviceStartModeInclude) app.Action(func(*kingpin.ParseContext) error { - var err error - - c.config.ServiceExclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", serviceExclude)) - if err != nil { - return fmt.Errorf("collector.service.exclude: %w", err) - } - - c.config.ServiceInclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", serviceInclude)) - if err != nil { - return fmt.Errorf("collector.service.include: %w", err) - } - c.config.ServiceStartModeInclude = strings.Split(serviceStartModeInclude, ",") return nil diff --git a/internal/collector/smtp/smtp.go b/internal/collector/smtp/smtp.go index 2bb36004..8978cd33 100644 --- a/internal/collector/smtp/smtp.go +++ b/internal/collector/smtp/smtp.go @@ -28,6 +28,7 @@ import ( "github.com/prometheus-community/windows_exporter/internal/pdh" pdhtypes "github.com/prometheus-community/windows_exporter/internal/pdh/types" "github.com/prometheus-community/windows_exporter/internal/types" + "github.com/prometheus-community/windows_exporter/internal/utils" "github.com/prometheus/client_golang/prometheus" ) @@ -119,33 +120,17 @@ func NewWithFlags(app *kingpin.Application) *Collector { config: ConfigDefaults, } - var serverExclude, serverInclude string - - app.Flag( + utils.RegexpFlag(app, "collector.smtp.server-exclude", "Regexp of virtual servers to exclude. Server name must both match include and not match exclude to be included.", - ).Default("").StringVar(&serverExclude) + "", &c.config.ServerExclude, + ) - app.Flag( + utils.RegexpFlag(app, "collector.smtp.server-include", "Regexp of virtual servers to include. Server name must both match include and not match exclude to be included.", - ).Default(".+").StringVar(&serverInclude) - - app.Action(func(*kingpin.ParseContext) error { - var err error - - c.config.ServerExclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", serverExclude)) - if err != nil { - return fmt.Errorf("collector.smtp.server-exclude: %w", err) - } - - c.config.ServerInclude, err = regexp.Compile(fmt.Sprintf("^(?:%s)$", serverInclude)) - if err != nil { - return fmt.Errorf("collector.smtp.server-include: %w", err) - } - - return nil - }) + ".+", &c.config.ServerInclude, + ) return c } diff --git a/internal/config/config.go b/internal/config/config.go index 988607bc..4776fdf8 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -189,6 +189,12 @@ func (c *Resolver) setDefault(v getFlagger) error { } if err := checkFlagValue(f, values); err != nil { + // Some flag values, like those of utils.RegexpFlag, name their flag + // in their errors, because kingpin doesn't for command line values. + if strings.HasPrefix(err.Error(), name+": ") { + return fmt.Errorf("line %d: %w", c.lines[name], err) + } + return fmt.Errorf("line %d: %s: %w", c.lines[name], name, err) } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 86aa5612..9514c357 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -548,6 +548,73 @@ func TestParseEmptyValue(t *testing.T) { } } +// TestRegexpFlags verifies that every include and exclude flag of the +// collectors accepts a value from the configuration file, and that an invalid +// expression fails the parsing with an error that names the flag. +func TestRegexpFlags(t *testing.T) { + t.Parallel() + + for _, name := range []string{ + "collector.iis.app-exclude", + "collector.iis.app-include", + "collector.iis.site-exclude", + "collector.iis.site-include", + "collector.logical_disk.volume-exclude", + "collector.logical_disk.volume-include", + "collector.net.nic-exclude", + "collector.net.nic-include", + "collector.physical_disk.disk-exclude", + "collector.physical_disk.disk-include", + "collector.printer.exclude", + "collector.printer.include", + "collector.process.exclude", + "collector.process.include", + "collector.scheduled_task.exclude", + "collector.scheduled_task.include", + "collector.service.exclude", + "collector.service.include", + "collector.smtp.server-exclude", + "collector.smtp.server-include", + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + app := kingpin.New("test", "test") + app.Flag("config.file", "config").String() + collector.NewWithFlags(app) + + require.NotNil(t, app.GetFlag(name)) + + err := config.Parse(app, []string{"--" + name + "=("}) + require.ErrorContains(t, err, name+": error parsing regexp: missing closing ): `^(?:()$`") + + parts := strings.SplitN(name, ".", 3) + path := filepath.Join(t.TempDir(), "config.yaml") + data := fmt.Sprintf("%s:\n %s:\n %s: \"foo|bar\"\n", parts[0], parts[1], parts[2]) + require.NoError(t, os.WriteFile(path, []byte(data), 0o600)) + + app = kingpin.New("test", "test") + app.Flag("config.file", "config").String() + collector.NewWithFlags(app) + + require.NoError(t, config.Parse(app, []string{"--config.file=" + path})) + require.Equal(t, "foo|bar", app.GetFlag(name).Model().Value.String()) + + // An invalid expression in the configuration file names its key and line once. + data = fmt.Sprintf("%s:\n %s:\n %s: \"(\"\n", parts[0], parts[1], parts[2]) + require.NoError(t, os.WriteFile(path, []byte(data), 0o600)) + + app = kingpin.New("test", "test") + app.Flag("config.file", "config").String() + collector.NewWithFlags(app) + + err = config.Parse(app, []string{"--config.file=" + path}) + require.ErrorContains(t, err, "line 3: "+name+": error parsing regexp: missing closing ): `^(?:()$`") + require.Equal(t, 1, strings.Count(err.Error(), name)) + }) + } +} + // TestParseFlagValues verifies that the configuration file takes the values the // flags accept, whatever the Go type of the configuration, and that an invalid // value fails with its key and line, even if the command line sets the flag. diff --git a/internal/utils/regexp.go b/internal/utils/regexp.go new file mode 100644 index 00000000..4fe57b6c --- /dev/null +++ b/internal/utils/regexp.go @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Copyright The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build windows + +package utils + +import ( + "fmt" + "regexp" + "slices" + + "github.com/alecthomas/kingpin/v2" + "github.com/prometheus-community/windows_exporter/internal/types" +) + +//nolint:gochecknoglobals +var ( + // matchAllExpressions are the expressions of an include filter that matches every name. + matchAllExpressions = []string{anchor(".+"), anchor(".*"), types.RegExpAny.String()} + // matchNoneExpressions are the expressions of an exclude filter that matches no name. + matchNoneExpressions = []string{anchor(""), types.RegExpEmpty.String()} +) + +// anchor wraps expr, so that it must match the whole input. +func anchor(expr string) string { + return "^(?:" + expr + ")$" +} + +// CompileAnchored compiles expr, so that it must match the whole input. +func CompileAnchored(expr string) (*regexp.Regexp, error) { + return regexp.Compile(anchor(expr)) +} + +// MustCompileAnchored is like [CompileAnchored], but panics if expr can't be parsed. +func MustCompileAnchored(expr string) *regexp.Regexp { + return regexp.MustCompile(anchor(expr)) +} + +// IsMatchAll reports whether re is an include filter that matches every name: +// the default of an include flag, [types.RegExpAny] or nil, which the New function of +// a collector replaces with the default. +func IsMatchAll(re *regexp.Regexp) bool { + return re == nil || slices.Contains(matchAllExpressions, re.String()) +} + +// IsMatchNone reports whether re is an exclude filter that matches no name: +// the default of an exclude flag, [types.RegExpEmpty] or nil, which the New function +// of a collector replaces with the default. +func IsMatchNone(re *regexp.Regexp) bool { + return re == nil || slices.Contains(matchNoneExpressions, re.String()) +} + +// anchoredRegexpValue is a [kingpin.Value] which compiles its value with [CompileAnchored]. +type anchoredRegexpValue struct { + name string + expr string + target **regexp.Regexp +} + +func (v *anchoredRegexpValue) Set(expr string) error { + re, err := CompileAnchored(expr) + if err != nil { + return fmt.Errorf("%s: %w", v.name, err) + } + + v.expr = expr + *v.target = re + + return nil +} + +func (v *anchoredRegexpValue) String() string { + return v.expr +} + +// RegexpFlag registers the flag name, whose value is a regular expression that +// must match the whole name. The compiled expression is stored in target when +// the flags are parsed, including the default and a value from the +// configuration file. An invalid expression fails the parsing with an error +// that starts with the name of the flag. +func RegexpFlag(app *kingpin.Application, name, help, defaultValue string, target **regexp.Regexp) *kingpin.FlagClause { + flag := app.Flag(name, help).Default(defaultValue) + flag.SetValue(&anchoredRegexpValue{name: name, target: target}) + + return flag +} diff --git a/internal/utils/regexp_test.go b/internal/utils/regexp_test.go new file mode 100644 index 00000000..89a69042 --- /dev/null +++ b/internal/utils/regexp_test.go @@ -0,0 +1,148 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Copyright The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build windows + +package utils_test + +import ( + "regexp" + "testing" + + "github.com/alecthomas/kingpin/v2" + "github.com/prometheus-community/windows_exporter/internal/types" + "github.com/prometheus-community/windows_exporter/internal/utils" + "github.com/stretchr/testify/require" +) + +func TestRegexpFlag(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + defaultValue string + args []string + want string + matches []string + notMatches []string + matchAll bool + matchNone bool + errorText string + }{ + { + name: "include default", defaultValue: ".+", want: "^(?:.+)$", + matches: []string{"a", "svc host"}, notMatches: []string{""}, matchAll: true, + }, + { + name: "exclude default", defaultValue: "", want: "^(?:)$", + matches: []string{""}, notMatches: []string{"a"}, matchNone: true, + }, + { + name: "match all with star", defaultValue: ".+", args: []string{"--collector.test.include=.*"}, + want: "^(?:.*)$", matchAll: true, + }, + { + name: "alternation is anchored as a whole", defaultValue: ".+", args: []string{"--collector.test.include=foo|bar"}, + want: "^(?:foo|bar)$", matches: []string{"foo", "bar"}, notMatches: []string{"foobar", "xfoo", "barx"}, + }, + { + name: "default from configuration", defaultValue: "/Microsoft/.+", want: "^(?:/Microsoft/.+)$", + matches: []string{"/Microsoft/Windows/Task"}, notMatches: []string{"/Vendor/Task"}, + }, + { + name: "invalid value names the flag", defaultValue: ".+", args: []string{"--collector.test.include=("}, + errorText: "collector.test.include: error parsing regexp: missing closing ): `^(?:()$`", + }, + { + name: "invalid default names the flag", defaultValue: "[", + errorText: "collector.test.include: error parsing regexp", + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + var re *regexp.Regexp + + app := kingpin.New("test", "test") + utils.RegexpFlag(app, "collector.test.include", "help", tc.defaultValue, &re) + + _, err := app.Parse(tc.args) + if tc.errorText != "" { + require.ErrorContains(t, err, tc.errorText) + + return + } + + require.NoError(t, err) + require.NotNil(t, re) + require.Equal(t, tc.want, re.String()) + require.Equal(t, tc.matchAll, utils.IsMatchAll(re)) + require.Equal(t, tc.matchNone, utils.IsMatchNone(re)) + + for _, name := range tc.matches { + require.True(t, re.MatchString(name), name) + } + + for _, name := range tc.notMatches { + require.False(t, re.MatchString(name), name) + } + }) + } +} + +func TestRegexpFlagValue(t *testing.T) { + t.Parallel() + + var re *regexp.Regexp + + app := kingpin.New("test", "test") + utils.RegexpFlag(app, "collector.test.exclude", "help", "", &re) + + _, err := app.Parse([]string{"--collector.test.exclude=a.b"}) + require.NoError(t, err) + require.Equal(t, "a.b", app.GetFlag("collector.test.exclude").Model().Value.String()) + require.Equal(t, []string{""}, app.GetFlag("collector.test.exclude").Model().Default) +} + +func TestIsMatchAllAndNone(t *testing.T) { + t.Parallel() + + require.True(t, utils.IsMatchAll(nil)) + require.True(t, utils.IsMatchAll(types.RegExpAny)) + require.True(t, utils.IsMatchAll(utils.MustCompileAnchored(".+"))) + require.True(t, utils.IsMatchAll(utils.MustCompileAnchored(".*"))) + require.False(t, utils.IsMatchAll(utils.MustCompileAnchored("foo"))) + require.False(t, utils.IsMatchAll(types.RegExpEmpty)) + + require.True(t, utils.IsMatchNone(nil)) + require.True(t, utils.IsMatchNone(types.RegExpEmpty)) + require.True(t, utils.IsMatchNone(utils.MustCompileAnchored(""))) + require.False(t, utils.IsMatchNone(utils.MustCompileAnchored("foo"))) + require.False(t, utils.IsMatchNone(types.RegExpAny)) +} + +func TestCompileAnchored(t *testing.T) { + t.Parallel() + + re, err := utils.CompileAnchored("svc") + require.NoError(t, err) + require.True(t, re.MatchString("svc")) + require.False(t, re.MatchString("svchost")) + + _, err = utils.CompileAnchored("(") + require.Error(t, err) + + require.Panics(t, func() { utils.MustCompileAnchored("(") }) +}