From 9147165ebcf3b3a013e0d0a97183a085d697be22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan-Otto=20Kr=C3=B6pke?= Date: Mon, 12 Oct 2026 21:40:44 +0200 Subject: [PATCH] scheduled_task: skip filtered folders, exclude Windows tasks and missed runs by default (#2700) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jan-Otto Kröpke Signed-off-by: Claude Co-authored-by: Claude --- UPGRADE.md | 15 ++ docs/collector.scheduled_task.md | 35 ++- docs/example_config.yml | 1 + internal/collector/scheduled_task/AGENTS.md | 58 +++++ .../collector/scheduled_task/folder_filter.go | 177 ++++++++++++++ .../scheduled_task/folder_filter_test.go | 215 ++++++++++++++++++ .../scheduled_task/scheduled_task.go | 111 ++++++--- .../scheduled_task_internal_test.go | 39 +++- .../scheduled_task_native_test.go | 146 ++++++++++-- internal/config/config_test.go | 34 +++ 10 files changed, 775 insertions(+), 56 deletions(-) create mode 100644 internal/collector/scheduled_task/folder_filter.go create mode 100644 internal/collector/scheduled_task/folder_filter_test.go diff --git a/UPGRADE.md b/UPGRADE.md index 3163f91f..f8f0387a 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -18,6 +18,7 @@ Most installations need no changes. Go through the checklist below and read the | You query `windows_gpu_info` | Replace the `phys` label with `device_number` | [GPU collector](#gpu-collector) | | You alert on BitLocker status | Replace `status="disabled"` with `status="off"` | [BitLocker status](#bitlocker-status) | | You query Hyper-V, logical disk queue, cache, IIS output cache, memory, scheduled task or terminal services metrics | Update queries | [Renamed, deprecated and retyped metrics](#renamed-deprecated-and-retyped-metrics) | +| You enable the `scheduled_task` collector and use tasks below `\Microsoft\` or missed runs | Set `--collector.scheduled_task.exclude` or `missed-runs` | [scheduled_task collector](#scheduled_task-collector) | | You chart SQL Server latencies, SQL Server lock counts or DHCP denials | Expect corrected values | [Corrected values](#corrected-values) | | You enable the `hyperv` or `mscluster` collector | Check the new default sub-collectors | [New default sub-collectors](#new-default-sub-collectors) | | You set scrape timeouts above 5 minutes or below the timeout margin | Review the new limits | [Scrape timeouts](#scrape-timeouts) | @@ -172,6 +173,19 @@ Other changes: - New metrics: `windows_container_start_time_seconds`, `windows_container_processes`, `windows_container_memory_page_faults_total` and `windows_container_storage_writable_layer_usage_bytes`. - On hosts without the Containers feature, the collector logs one warning at startup instead of failing every scrape. +## scheduled_task collector + +`--collector.scheduled_task.exclude` now defaults to `/Microsoft/.+`. The tasks of Windows itself, below `\Microsoft\`, are no longer reported unless you set the flag. +They are most of the tasks and folders of an installation, and skipping their folders makes a scrape several times faster. + +| You want | Set | +|-----------------------------------|------------------------------------------------------------------------------------------------------------------| +| Every task, as in 0.31 | `--collector.scheduled_task.exclude=""`, or `exclude: ""` in the configuration file | +| Windows tasks except some folders | The folders to exclude, e.g. `--collector.scheduled_task.exclude="/Microsoft/Windows/(?:Defrag\|Diagnosis)/.+"` | + +`windows_scheduled_task_missed_runs` is only reported with the new `--collector.scheduled_task.missed-runs` flag (`missed-runs: true` in the configuration file). +Reading it costs two Task Scheduler requests per task and scrape, about a quarter of a scrape. + ## GPU collector - The `phys` label of `windows_gpu_info` was renamed to `device_number`. It holds the PCI device number; `phys` in the other GPU metrics is the physical adapter index. @@ -326,6 +340,7 @@ These changes affect Go programs that embed `github.com/prometheus-community/win - `container.Config.ContainerDStateDir` was replaced by `container.Config.CRIEndpoint`. See [container collector](#container-collector). - `process.Config.CounterVersion` was removed. See [process collector](#process-collector). - New field in `collector.Config`: `StorageSpaces`. +- `scheduled_task.ConfigDefaults.TaskExclude`, also used when `scheduled_task.Config.TaskExclude` is nil, now excludes `/Microsoft/.+`, like the flag. New field `scheduled_task.Config.MissedRuns`, false by default. - `collector.Config.ThermalZone` was removed together with the `thermalzone` collector. - New method `Collection.NewHandlerWithContext`. `Collection.NewHandler` is unchanged. - `Collection.Close` now releases all collector resources, including PDH queries and worker goroutines. Call it when you discard a collection, for example on a configuration reload. diff --git a/docs/collector.scheduled_task.md b/docs/collector.scheduled_task.md index d0acec00..02987170 100644 --- a/docs/collector.scheduled_task.md +++ b/docs/collector.scheduled_task.md @@ -18,9 +18,36 @@ E.G. `--collector.scheduled_task.include="Firefox.*"` ### `--collector.scheduled_task.exclude` -If given, the path of the task needs to *not* match the exclude regular expression in order for the corresponding metrics to be reported. Default: empty +If given, the path of the task needs to *not* match the exclude regular expression in order for the corresponding metrics to be reported. Default: `/Microsoft/.+` -E.G. `--collector.scheduled_task.exclude="/Microsoft/.+"` +The default excludes the tasks of Windows itself, below `/Microsoft/`. They are +most of the tasks and folders of an installation and rarely worth alerting on. +To report every task, set the flag to an empty value: + +E.G. `--collector.scheduled_task.exclude=""`, or `exclude: ""` below +`collector.scheduled_task` in the configuration file. A key without a value +keeps the default. + +To report Windows tasks except some folders, list the folders to exclude +instead. Tasks in folders that are not listed are reported, including folders +that a Windows update adds: + +E.G. `--collector.scheduled_task.exclude="/Microsoft/Windows/(?:Defrag|Diagnosis|WindowsUpdate)/.+"` + +Each task folder costs the Task Scheduler service two requests per scrape. The +collector does not read folders whose tasks the include and exclude expressions +reject whatever their names are, such as everything below `/Microsoft/` with +the default. Expressions of the form `/Folder/.+` or `/Folder/.*` (also as +alternatives) are recognized for exclude, and a literal folder prefix for include. +Errors reading folders that are not walked are not reported. + +### `--collector.scheduled_task.missed-runs` + +If set, the collector reads and reports `windows_scheduled_task_missed_runs`. +Reading the missed runs of a task costs the Task Scheduler service two requests +per scrape, about a quarter of a scrape, and the service lists the start time +of every missed run to count them. Without this flag, the dashboards show 0 missed runs +and an empty table of tasks with missed runs. Default: `false` ## Metrics @@ -28,7 +55,7 @@ Name | Description | Type | Labels -----|-------------|------|------- `windows_scheduled_task_last_result` | **Deprecated:** use `windows_scheduled_task_last_result_status`. 1 if the last result code is zero, 0 otherwise; omitted for tasks that have never run | gauge | task `windows_scheduled_task_last_result_status` | The last result status of a scheduled task, 1 if the current status, 0 otherwise | gauge | task, status -`windows_scheduled_task_missed_runs` | The number of times the registered task missed a scheduled run | gauge | task +`windows_scheduled_task_missed_runs` | The number of times the registered task missed a scheduled run; only with `--collector.scheduled_task.missed-runs` | gauge | task `windows_scheduled_task_state` | The current state of a scheduled task | gauge | task, state For the values of the `state` and `status` labels, see below. @@ -86,6 +113,8 @@ A task can be in the following states: ### Example metric +This Windows task is only reported with `--collector.scheduled_task.exclude=""`. + ``` windows_scheduled_task_last_result{task="/Microsoft/Windows/Chkdsk/SyspartRepair"} 1 windows_scheduled_task_last_result_status{status="success",task="/Microsoft/Windows/Chkdsk/SyspartRepair"} 1 diff --git a/docs/example_config.yml b/docs/example_config.yml index 62f3ab2c..08c1323f 100644 --- a/docs/example_config.yml +++ b/docs/example_config.yml @@ -7,6 +7,7 @@ collector: include: "windows_exporter" scheduled_task: include: /Microsoft/.+ + exclude: "" log: level: debug scrape: diff --git a/internal/collector/scheduled_task/AGENTS.md b/internal/collector/scheduled_task/AGENTS.md index 2c527366..ea47aa13 100644 --- a/internal/collector/scheduled_task/AGENTS.md +++ b/internal/collector/scheduled_task/AGENTS.md @@ -20,6 +20,16 @@ which an unprivileged user can read. - Read only the properties that are published, apply the include/exclude filter to the path before any service read, and keep property reads conditional on whether the metric that uses them is emitted. +- `folderFilter` skips subfolders whose tasks the include/exclude expressions + reject whatever their names are, by analyzing the parsed expressions. It must + stay conservative: when in doubt, walk the folder. A Windows 11 desktop has + about 150 folders for about 200 tasks, and listing the tasks and subfolders of + each costs ~0.15 ms per call, so `exclude=/Microsoft/.+` saves most of a scrape. + The default exclude is `/Microsoft/.+` for that reason; keep `ConfigDefaults` + and the flag default in sync through `defaultTaskExclude`. +- `taskReader` carries the task and folder filters and which properties to + read. `NumberOfMissedRuns` is only read and published with `missed-runs` + (off by default), which saves its two RPCs per task. - The fake native task in `scheduled_task_native_test.go` counts reads per vtable slot; extend those assertions when adding a property read. - To find slow calls, time each call type over a full enumeration on a live @@ -30,3 +40,51 @@ which an unprivileged user can read. scrape through `internal/metriccache` while a task's path, state, last result and missed runs are unchanged. Add new inputs of a task's metrics to `taskValues`, or the cache returns stale series. + +## Measured costs and rejected approaches + +Measured in October 2026 on a Windows 11 24H2 desktop (taskschd.dll +10.0.26100.5074), unprivileged: 207 tasks in 150 folders (134 leaf folders, 36 +of them empty), about 150 ms per scrape without filters. + +- Per call: `GetTasks` ~170 µs and `GetFolders` ~145 µs per folder; `State` and + `LastTaskResult` ~130 µs per task; `NumberOfMissedRuns` ~310 µs per task + (~560 µs for disabled tasks). `Path` and folder `Path` take 1–2 µs (local). + `NumberOfMissedRuns` alone is about 28% of a scrape. +- `NumberOfMissedRuns` is computed with two RPCs: `SchRpcGetLastRunInfo` + (opnum 16), then `SchRpcScheduledRuntimes` (opnum 15) from the start of the last run + to the current local time with an unlimited `cRequested`. The service + computes every scheduled run in that window from the task definition and + returns all of them, so tasks with many missed runs (13,146 seen) send large + responses each scrape. Tasks without time triggers return + `SCHED_S_TASK_NOT_SCHEDULED`. `LastTaskResult` also calls + `SchRpcGetLastRunInfo`, so reading both costs one duplicate round trip. + `SchRpcGetNumberOfMissedRuns` (opnum 18) always returns 0 and is not what COM + uses. +- Missed runs are rare but real: 6 of 207 tasks were non-zero, 2 of them + disabled. Skipping the read for disabled tasks would change published values. +- A persistent COM connection (dedicated thread, service kept connected) saves + ~1.7 ms per scrape. `CoCreateInstance` and `Connect` are cheap; the service's + per-call time dominates. +- Direct MS-TSCH RPC over `ncalrpc` (hand-marshalled NDR, `I_RpcSendReceive`) + is only 5–8 µs faster per call. It would only save the duplicate + `SchRpcGetLastRunInfo` (~18 ms per scrape, ~12%) plus ~9 ms of client CPU, at + the cost of a hand-written NDR client. Not worth it. Note that + `SchRpcGetTaskInfo` only returns the state with `SCH_FLAG_STATE` + (`0x10000000`). +- One `SchRpcEnumInstances` for all running tasks took ~21 ms, slower than the + per-task state reads. +- There is no flat or recursive task enumeration: `GetTasks`/`GetFolders` and + `SchRpcEnumTasks`/`SchRpcEnumFolders` list one folder each, with only the + hidden-task flag. The registry `TaskCache` is undocumented and not readable + without admin rights. +- `MSFT_ScheduledTask` (CIM, `root/Microsoft/Windows/TaskScheduler`) returns a + flat list with path and state, but is about 3× slower: ~440 ms through + `internal/mi` in-process for the same 207 tasks, whatever properties are + selected, plus ~120 ms for last result and missed runs. The work moves into + `WmiPrvSE` (up to ~30% of one core while querying). +- `missed_runs` is opt-in through `missed-runs`. Not taken yet: a cache + duration that serves the last task list. +- Profiling the exporter with pprof on Windows attributes the time blocked in + these COM calls to `runtime.cgocall`; that is waiting, not exporter CPU. The + Schedule service's own CPU needs an elevated reader of its process times. diff --git a/internal/collector/scheduled_task/folder_filter.go b/internal/collector/scheduled_task/folder_filter.go new file mode 100644 index 00000000..6b0b3628 --- /dev/null +++ b/internal/collector/scheduled_task/folder_filter.go @@ -0,0 +1,177 @@ +// 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 scheduled_task + +import ( + "regexp" + "regexp/syntax" + "strings" +) + +// folderFilter skips task folders whose tasks the include and exclude filters +// reject whatever their names are. Listing the tasks and subfolders of a folder +// costs two round trips to the Task Scheduler service, so a filter such as +// exclude="/Microsoft/.+" saves the walk of most folders of a Windows install. +// +// The analysis is conservative: it skips a folder only when the expression +// proves that no task below it passes, and keeps every folder otherwise. Task +// paths cannot contain line breaks, because tasks are stored as files below +// %SystemRoot%\System32\Tasks, so "." matches every character of a path. +type folderFilter struct { + // include and exclude are nil when the expression gives no folder information. + include *syntax.Regexp + exclude *syntax.Regexp +} + +func newFolderFilter(include, exclude *regexp.Regexp) folderFilter { + return folderFilter{include: parseFilter(include), exclude: parseFilter(exclude)} +} + +func parseFilter(re *regexp.Regexp) *syntax.Regexp { + if re == nil { + return nil + } + + // regexp.Compile parses with syntax.Perl. + parsed, err := syntax.Parse(re.String(), syntax.Perl) + if err != nil { + return nil + } + + return parsed.Simplify() +} + +// includeFolder reports whether a task below the folder with the given +// slash-separated path may pass both filters. +func (f folderFilter) includeFolder(path string) bool { + prefix := strings.TrimSuffix(path, "/") + "/" + + if f.include != nil && !mayMatchPrefixed([]*syntax.Regexp{f.include}, prefix, false) { + return false + } + + return f.exclude == nil || !matchesAllPrefixed([]*syntax.Regexp{f.exclude}, prefix, true) +} + +// mayMatchPrefixed reports false only if no string that starts with prefix +// matches the sequence of expressions. Without a start anchor a match can begin +// anywhere, so only anchored expressions rule out a prefix. +func mayMatchPrefixed(seq []*syntax.Regexp, prefix string, anchored bool) bool { + for i, re := range seq { + if prefix == "" { + return true + } + + switch re.Op { + case syntax.OpBeginText, syntax.OpBeginLine: + if i == 0 && !anchored { + anchored = true + + continue + } + + return true + case syntax.OpCapture, syntax.OpConcat: + return mayMatchPrefixed(splice(re.Sub, seq[i+1:]), prefix, anchored) + case syntax.OpAlternate: + for _, sub := range re.Sub { + if mayMatchPrefixed(splice([]*syntax.Regexp{sub}, seq[i+1:]), prefix, anchored) { + return true + } + } + + return false + case syntax.OpLiteral: + if !anchored || re.Flags&syntax.FoldCase != 0 { + return true + } + + literal := string(re.Rune) + if !strings.HasPrefix(prefix, literal) { + return strings.HasPrefix(literal, prefix) + } + + prefix = prefix[len(literal):] + case syntax.OpEndText, syntax.OpEndLine: + // Task paths continue after the folder prefix and have no line breaks. + return !anchored + default: + return true + } + } + + return true +} + +// matchesAllPrefixed reports true only if every string that starts with prefix +// and continues with at least one character matches the sequence of expressions. +// A full match implies an unanchored match, so start anchors are not required. +func matchesAllPrefixed(seq []*syntax.Regexp, prefix string, atStart bool) bool { + for i, re := range seq { + switch re.Op { + case syntax.OpBeginText, syntax.OpBeginLine: + if atStart { + continue + } + + return false + case syntax.OpCapture, syntax.OpConcat: + return matchesAllPrefixed(splice(re.Sub, seq[i+1:]), prefix, atStart) + case syntax.OpAlternate: + for _, sub := range re.Sub { + if matchesAllPrefixed(splice([]*syntax.Regexp{sub}, seq[i+1:]), prefix, atStart) { + return true + } + } + + return false + case syntax.OpLiteral: + literal := string(re.Rune) + if re.Flags&syntax.FoldCase != 0 || !strings.HasPrefix(prefix, literal) { + return false + } + + prefix = prefix[len(literal):] + atStart = atStart && literal == "" + case syntax.OpStar, syntax.OpPlus: + // ".*" or ".+" as the last expression matches the rest of the path, + // which is never empty. + op := re.Sub[0].Op + + return (op == syntax.OpAnyChar || op == syntax.OpAnyCharNotNL) && onlyEndAnchors(seq[i+1:]) + default: + return false + } + } + + return false +} + +func onlyEndAnchors(seq []*syntax.Regexp) bool { + for _, re := range seq { + if re.Op != syntax.OpEndText && re.Op != syntax.OpEndLine { + return false + } + } + + return true +} + +func splice(head, tail []*syntax.Regexp) []*syntax.Regexp { + return append(append(make([]*syntax.Regexp, 0, len(head)+len(tail)), head...), tail...) +} diff --git a/internal/collector/scheduled_task/folder_filter_test.go b/internal/collector/scheduled_task/folder_filter_test.go new file mode 100644 index 00000000..1f04a053 --- /dev/null +++ b/internal/collector/scheduled_task/folder_filter_test.go @@ -0,0 +1,215 @@ +// 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 scheduled_task + +import ( + "fmt" + "regexp" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestFolderFilter(t *testing.T) { + t.Parallel() + + // The flags anchor the expressions like this. + flag := func(expr string) *regexp.Regexp { + return regexp.MustCompile(fmt.Sprintf("^(?:%s)$", expr)) + } + + for _, tc := range []struct { + name string + include *regexp.Regexp + exclude *regexp.Regexp + included []string + skipped []string + }{ + { + name: "defaults", + include: ConfigDefaults.TaskInclude, + exclude: ConfigDefaults.TaskExclude, + included: []string{"/Other"}, + skipped: []string{"/Microsoft", "/Microsoft/Windows"}, + }, + { + name: "flag defaults", + include: flag(".+"), + exclude: flag(defaultTaskExclude), + included: []string{"/Other"}, + skipped: []string{"/Microsoft", "/Microsoft/Windows"}, + }, + { + name: "empty exclude", + include: flag(".+"), + exclude: flag(""), + included: []string{"/Microsoft", "/Other"}, + }, + { + name: "nil filters", + included: []string{"/Microsoft"}, + }, + { + name: "exclude subtree", + include: flag(".+"), + exclude: flag("/Microsoft/.+"), + included: []string{"/Other", "/MicrosoftEdge", "/Microsofx"}, + skipped: []string{"/Microsoft", "/Microsoft/Windows", "/Microsoft/Windows/Defrag"}, + }, + { + name: "exclude deeper subtree", + include: flag(".+"), + exclude: flag("/Microsoft/Windows/.*"), + included: []string{"/Microsoft", "/Microsoft/Office", "/Microsoft/WindowsUpdate"}, + skipped: []string{"/Microsoft/Windows", "/Microsoft/Windows/Defrag"}, + }, + { + name: "exclude alternatives", + include: flag(".+"), + exclude: flag("/Microsoft/.*|/Mozilla/.+"), + included: []string{"/Other", "/Mozillas"}, + skipped: []string{"/Microsoft/Windows", "/Mozilla", "/Mozilla/Firefox"}, + }, + { + name: "exclude capture group", + exclude: regexp.MustCompile(`^(/Microsoft/.*)$`), + included: []string{"/Other"}, + skipped: []string{"/Microsoft/Windows"}, + }, + { + name: "unanchored exclude", + exclude: regexp.MustCompile(`/Microsoft/.*`), + included: []string{"/Other", "/Vendor"}, + skipped: []string{"/Microsoft"}, + }, + { + name: "exclude every task", + exclude: flag(".*"), + skipped: []string{"/Microsoft", "/Other"}, + }, + { + name: "exclude without a full subtree", + exclude: flag( + "/Microsoft/[^/]+|/Microsoft/Windows/.+/Task|/Microsoft/Windows|(?i)/microsoft/.+|/Other/.+x|/Other/\n.+|/Other/(?:^.+)", + ), + included: []string{"/Microsoft", "/Microsoft/Windows", "/Other"}, + }, + { + name: "include subtree", + include: flag("/Vendor/App/.+"), + included: []string{"/Vendor", "/Vendor/App", "/Vendor/App/Sub"}, + skipped: []string{"/Microsoft", "/Vendor/Other", "/Vendo"}, + }, + { + name: "include alternatives", + include: flag("/Vendor/.+|/Other/Task"), + included: []string{"/Vendor", "/Other"}, + skipped: []string{"/Microsoft", "/Other/Sub"}, + }, + { + name: "include with folder prefix", + include: flag("/Vend.*"), + included: []string{"/Vendor", "/Vendor/App", "/Vend"}, + skipped: []string{"/Microsoft"}, + }, + { + name: "unanchored include", + include: regexp.MustCompile(`Firefox`), + included: []string{"/Microsoft", "/Mozilla"}, + }, + { + name: "case-insensitive include", + include: flag("(?i)/vendor/.+"), + included: []string{"/Vendor", "/Microsoft"}, + }, + { + name: "include and exclude", + include: flag("/Microsoft/.+"), + exclude: flag("/Microsoft/Windows/.+"), + included: []string{"/Microsoft", "/Microsoft/Office"}, + skipped: []string{"/Other", "/Microsoft/Windows", "/Microsoft/Windows/Defrag"}, + }, + { + name: "exclude listed subfolders", + exclude: flag("/Microsoft/Windows/(?:Defrag|Diagnosis|WindowsUpdate)/.+"), + included: []string{"/Microsoft", "/Microsoft/Windows", "/Microsoft/Windows/Other", "/Other"}, + skipped: []string{"/Microsoft/Windows/Defrag", "/Microsoft/Windows/Diagnosis", "/Microsoft/Windows/WindowsUpdate"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + filter := newFolderFilter(tc.include, tc.exclude) + + for _, path := range tc.included { + require.True(t, filter.includeFolder(path), path) + } + + for _, path := range tc.skipped { + require.False(t, filter.includeFolder(path), path) + } + }) + } +} + +// TestFolderFilterSkipsOnlyFilteredTasks checks the folder decision against the +// task filters for task paths below each folder. +func TestFolderFilterSkipsOnlyFilteredTasks(t *testing.T) { + t.Parallel() + + expressions := []string{ + "", ".+", ".*", "/Microsoft/.+", "/Microsoft/.*", "/Microsoft/Windows/.+", "/Microsoft/.+|/Other/.*", + "/Microsoft/Windows", "/Microsoft/[^/]+", "/Micro.*", "(?i)/microsoft/.+", "/Other/Task", "/Other/.+|", + `\A/Microsoft/.+\z`, "(?m)^/Microsoft/.*$", "(?s)/Microsoft/.+", "(/Microsoft/.+)", "^/Micro|/Other/.+", + "/Microsoft/Windows/(?:Defrag|Diagnosis)/.+", "/Microsoft/Windows/Defrag/Task", + } + folders := []string{ + "/Microsoft", "/Microsoft/Windows", "/Microsoft/Windows/Defrag", "/Microsoft/Windows/Diagnosis", "/MicrosoftEdge", "/Other", + } + names := []string{"Task", "Sub/Task", "x"} + + // The flags anchor the expressions, Config values passed to New need not be. + regexps := make([]*regexp.Regexp, 0, 2*len(expressions)) + for _, expr := range expressions { + regexps = append(regexps, regexp.MustCompile(fmt.Sprintf("^(?:%s)$", expr)), regexp.MustCompile(expr)) + } + + skipped := 0 + + for _, include := range regexps { + for _, exclude := range regexps { + c := New(&Config{TaskInclude: include, TaskExclude: exclude}) + filter := newFolderFilter(c.config.TaskInclude, c.config.TaskExclude) + + for _, folder := range folders { + if filter.includeFolder(folder) { + continue + } + + skipped++ + + for _, name := range names { + path := folder + "/" + name + require.False(t, c.includeTask(path), "include %q exclude %q path %q", include, exclude, path) + } + } + } + } + + require.NotZero(t, skipped) +} diff --git a/internal/collector/scheduled_task/scheduled_task.go b/internal/collector/scheduled_task/scheduled_task.go index 4c7688cb..d36df524 100644 --- a/internal/collector/scheduled_task/scheduled_task.go +++ b/internal/collector/scheduled_task/scheduled_task.go @@ -23,6 +23,7 @@ import ( "log/slog" "regexp" "runtime" + "strconv" "strings" "time" @@ -40,12 +41,18 @@ const Name = "scheduled_task" type Config struct { TaskExclude *regexp.Regexp `yaml:"exclude"` TaskInclude *regexp.Regexp `yaml:"include"` + MissedRuns bool `yaml:"missed-runs"` } +// defaultTaskExclude excludes the tasks of Windows itself, which are most of +// the tasks and folders of an installation. +const defaultTaskExclude = "/Microsoft/.+" + //nolint:gochecknoglobals var ConfigDefaults = Config{ - TaskExclude: types.RegExpEmpty, + TaskExclude: regexp.MustCompile("^(?:" + defaultTaskExclude + ")$"), TaskInclude: types.RegExpAny, + MissedRuns: false, } type Collector struct { @@ -57,6 +64,9 @@ type Collector struct { missedRuns *prometheus.Desc state *prometheus.Desc + // folderFilter skips folders that cannot hold tasks passing the filters. + folderFilter folderFilter + // metricCache reuses the metrics of tasks whose values did not change since // the last scrape. Each task publishes 19 series, and building them is most // of the allocations of a scrape. @@ -139,14 +149,21 @@ func NewWithFlags(app *kingpin.Application) *Collector { app.Flag( "collector.scheduled_task.exclude", - "Regexp of tasks to exclude. Task path must both match include and not match exclude to be included.", - ).Default("").StringVar(&taskExclude) + "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) app.Flag( "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) + app.Flag( + "collector.scheduled_task.missed-runs", + "Read and publish windows_scheduled_task_missed_runs. "+ + "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 @@ -208,6 +225,8 @@ func (c *Collector) Build(logger *slog.Logger, _ *mi.Session) error { nil, ) + c.folderFilter = newFolderFilter(c.config.TaskInclude, c.config.TaskExclude) + // Cached metrics refer to the descriptors of an earlier Build. c.metricCache.Reset() @@ -228,7 +247,7 @@ var TASK_RESULT_STATUSES = []string{ } func (c *Collector) collect(ch chan<- prometheus.Metric) error { - scheduledTasks, err := getScheduledTasks(c.includeTask) + scheduledTasks, err := c.taskReader().getScheduledTasks() if errors.Is(err, errTasksSkipped) { // Tasks and folders that can't be read are skipped, the other tasks are still collected. c.logger.Warn("failed to read some scheduled tasks", @@ -248,6 +267,14 @@ func (c *Collector) includeTask(path string) bool { return c.config.TaskInclude.MatchString(path) && !c.config.TaskExclude.MatchString(path) } +func (c *Collector) taskReader() taskReader { + return taskReader{ + includeTask: c.includeTask, + includeFolder: c.folderFilter.includeFolder, + missedRuns: c.config.MissedRuns, + } +} + func (c *Collector) collectMetrics(ch chan<- prometheus.Metric, scheduledTasks ScheduledTasks) { scrape := c.metricCache.Begin() @@ -317,27 +344,39 @@ func (c *Collector) taskMetrics(task ScheduledTask) []prometheus.Metric { lastResult = 1.0 } - return append(metrics, - prometheus.MustNewConstMetric( - c.lastResult, - prometheus.GaugeValue, - lastResult, - task.Path, - ), - prometheus.MustNewConstMetric( - c.missedRuns, - prometheus.GaugeValue, - task.MissedRunsCount, - task.Path, - ), - ) + metrics = append(metrics, prometheus.MustNewConstMetric( + c.lastResult, + prometheus.GaugeValue, + lastResult, + task.Path, + )) + + if !c.config.MissedRuns { + return metrics + } + + return append(metrics, prometheus.MustNewConstMetric( + c.missedRuns, + prometheus.GaugeValue, + task.MissedRunsCount, + task.Path, + )) } // errTasksSkipped accompanies readable tasks when some tasks or folders could not be read. var errTasksSkipped = errors.New("tasks skipped") -// getScheduledTasks returns the tasks whose slash-separated paths pass include. -func getScheduledTasks(include func(path string) bool) (ScheduledTasks, error) { +// taskReader selects the tasks and folders to read and the task properties to read. +type taskReader struct { + // includeTask and includeFolder take slash-separated paths. + includeTask func(path string) bool + includeFolder func(path string) bool + missedRuns bool +} + +// getScheduledTasks returns the tasks that pass includeTask. It does not walk +// the subfolders that fail includeFolder. +func (r taskReader) getScheduledTasks() (ScheduledTasks, error) { // COM initialization and every interface call stay on the same OS thread. runtime.LockOSThread() @@ -366,7 +405,7 @@ func getScheduledTasks(include func(path string) bool) (ScheduledTasks, error) { defer root.Release() tasks := ScheduledTasks{} - if err := fetchTasksRecursively(root, `\`, include, &tasks); err != nil { + if err := r.fetchTasksRecursively(root, `\`, &tasks); err != nil { return tasks, fmt.Errorf("%w: %w", errTasksSkipped, err) } @@ -374,7 +413,7 @@ func getScheduledTasks(include func(path string) bool) (ScheduledTasks, error) { } // fetchTasksInFolder appends readable tasks and reports errors after reading the remaining tasks. -func fetchTasksInFolder(folder *taskschd.TaskFolder, include func(path string) bool, scheduledTasks *ScheduledTasks) error { +func (r taskReader) fetchTasksInFolder(folder *taskschd.TaskFolder, scheduledTasks *ScheduledTasks) error { tasks, err := folder.Tasks() if err != nil { return fmt.Errorf("get tasks: %w", err) @@ -390,7 +429,7 @@ func fetchTasksInFolder(folder *taskschd.TaskFolder, include func(path string) b continue } - parsedTask, ok, err := parseTask(task, include) + parsedTask, ok, err := r.parseTask(task) if err != nil { errs = append(errs, fmt.Errorf("parse task: %w", err)) @@ -408,11 +447,11 @@ func fetchTasksInFolder(folder *taskschd.TaskFolder, include func(path string) b } // fetchTasksRecursively appends readable tasks, retaining errors from skipped folders or tasks. -func fetchTasksRecursively( - folder *taskschd.TaskFolder, folderPath string, include func(path string) bool, scheduledTasks *ScheduledTasks, -) error { +// Subfolders that fail includeFolder are not walked, which saves two round trips +// to the Task Scheduler service per folder. +func (r taskReader) fetchTasksRecursively(folder *taskschd.TaskFolder, folderPath string, scheduledTasks *ScheduledTasks) error { errs := []error{} - if err := fetchTasksInFolder(folder, include, scheduledTasks); err != nil { + if err := r.fetchTasksInFolder(folder, scheduledTasks); err != nil { errs = append(errs, fmt.Errorf("folder %s: %w", folderPath, err)) } @@ -429,12 +468,14 @@ func fetchTasksRecursively( continue } - subfolderPath := folderPath - if path, err := subfolder.Path(); err == nil { - subfolderPath = path + subfolderPath, err := subfolder.Path() + if err != nil { + subfolderPath = folderPath + } else if !r.includeFolder(strings.ReplaceAll(subfolderPath, "\\", "/")) { + continue } - if err := fetchTasksRecursively(subfolder, subfolderPath, include, scheduledTasks); err != nil { + if err := r.fetchTasksRecursively(subfolder, subfolderPath, scheduledTasks); err != nil { errs = append(errs, err) } } @@ -442,18 +483,18 @@ func fetchTasksRecursively( return errors.Join(errs...) } -// parseTask reads a task that passes include and reports false for filtered +// parseTask reads a task that passes includeTask and reports false for filtered // tasks. Reading a task property other than Path is a round trip to the Task // Scheduler service, so filtered tasks are skipped before those reads, and only // properties that collectMetrics publishes are read. -func parseTask(task *taskschd.RegisteredTask, include func(path string) bool) (ScheduledTask, bool, error) { +func (r taskReader) parseTask(task *taskschd.RegisteredTask) (ScheduledTask, bool, error) { path, err := task.Path() if err != nil { return ScheduledTask{}, false, fmt.Errorf("get task path: %w", err) } parsedTask := ScheduledTask{Path: strings.ReplaceAll(path, "\\", "/")} - if !include(parsedTask.Path) { + if !r.includeTask(parsedTask.Path) { return ScheduledTask{}, false, nil } @@ -472,7 +513,7 @@ func parseTask(task *taskschd.RegisteredTask, include func(path string) bool) (S parsedTask.LastTaskResult = TaskResult(result) // collectMetrics omits missed runs for tasks that have not run. - if parsedTask.LastTaskResult != SCHED_S_TASK_HAS_NOT_RUN { + if r.missedRuns && parsedTask.LastTaskResult != SCHED_S_TASK_HAS_NOT_RUN { missed, err := task.NumberOfMissedRuns() if err != nil { return ScheduledTask{}, false, fmt.Errorf("get task missed runs: %w", err) diff --git a/internal/collector/scheduled_task/scheduled_task_internal_test.go b/internal/collector/scheduled_task/scheduled_task_internal_test.go index 9f616db8..42bcc3b8 100644 --- a/internal/collector/scheduled_task/scheduled_task_internal_test.go +++ b/internal/collector/scheduled_task/scheduled_task_internal_test.go @@ -23,6 +23,7 @@ import ( "strings" "testing" + "github.com/alecthomas/kingpin/v2" "github.com/prometheus/client_golang/prometheus" dto "github.com/prometheus/client_model/go" "github.com/prometheus/common/expfmt" @@ -113,7 +114,7 @@ func TestCollectMetrics(t *testing.T) { MissedRunsCount: 3, LastTaskResult: TaskResult(tc.result), } - families := gatherTaskMetrics(t, nil, ScheduledTasks{task}) + families := gatherTaskMetrics(t, &Config{MissedRuns: true}, ScheduledTasks{task}) states := families["windows_scheduled_task_state"] require.NotNil(t, states) @@ -243,7 +244,7 @@ func TestCollectMetricsCache(t *testing.T) { {Path: "/Added", State: TASK_STATE_QUEUED, LastTaskResult: SCHED_S_TASK_QUEUED}, } - c := New(nil) + c := New(&Config{MissedRuns: true}) require.NoError(t, c.Build(slog.New(slog.DiscardHandler), nil)) var previous map[string][]prometheus.Metric @@ -387,3 +388,37 @@ func benchmarkTasks(n int) ScheduledTasks { return tasks } + +func TestNewWithFlagsExclude(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + args []string + update bool + defrag bool + }{ + {name: "default"}, + {name: "empty", args: []string{"--collector.scheduled_task.exclude="}, update: true, defrag: true}, + {name: "empty separate value", args: []string{"--collector.scheduled_task.exclude", ""}, update: true, defrag: true}, + { + name: "listed folders", + args: []string{"--collector.scheduled_task.exclude=/Microsoft/Windows/(?:Defrag|Diagnosis)/.+"}, + update: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + app := kingpin.New("test", "test") + c := NewWithFlags(app) + + _, err := app.Parse(tc.args) + require.NoError(t, err) + + require.True(t, c.includeTask("/Vendor/Task")) + require.Equal(t, tc.update, c.includeTask("/Microsoft/Windows/UpdateOrchestrator/Schedule Scan")) + require.Equal(t, tc.defrag, c.includeTask("/Microsoft/Windows/Defrag/ScheduledDefrag")) + }) + } +} diff --git a/internal/collector/scheduled_task/scheduled_task_native_test.go b/internal/collector/scheduled_task/scheduled_task_native_test.go index 219a95be..641e3c56 100644 --- a/internal/collector/scheduled_task/scheduled_task_native_test.go +++ b/internal/collector/scheduled_task/scheduled_task_native_test.go @@ -18,6 +18,8 @@ package scheduled_task import ( + "fmt" + "log/slog" "regexp" "slices" "strings" @@ -46,7 +48,7 @@ func TestFetchTasksInFolderPartialCollection(t *testing.T) { folder := fakeTaskFolder(collection, nil, nil) tasks := ScheduledTasks{} - err := fetchTasksInFolder(folder, includeAllTasks, &tasks) + err := readAllTasks.fetchTasksInFolder(folder, &tasks) require.ErrorIs(t, err, ole.HRESULT(0x80070005)) require.Equal(t, 1, collectionsReleased) @@ -84,7 +86,7 @@ func TestFetchTasksRecursivelyPartialFolderCollection(t *testing.T) { root := fakeTaskFolder(emptyTasks, folders, nil) tasks := ScheduledTasks{} - err := fetchTasksRecursively(root, `\`, includeAllTasks, &tasks) + err := readAllTasks.fetchTasksRecursively(root, `\`, &tasks) require.ErrorIs(t, err, ole.HRESULT(0x80070005)) require.Equal(t, []int64{1, 2, 3}, *attempted) require.Len(t, tasks, 2) @@ -95,13 +97,76 @@ func TestFetchTasksRecursivelyPartialFolderCollection(t *testing.T) { require.Equal(t, 6, collectionsReleased) } +func TestFetchTasksRecursivelySkipsFilteredFolders(t *testing.T) { + for _, tc := range []struct { + name string + config *Config + paths []string + listedMicrosoft int + }{ + {name: "default exclude", paths: []string{"/Task", "/Vendor/Task"}}, + { + name: "empty exclude", + config: &Config{TaskExclude: regexp.MustCompile(`^(?:)$`)}, + paths: []string{"/Task", "/Microsoft/Task", "/Vendor/Task"}, listedMicrosoft: 2, + }, + } { + t.Run(tc.name, func(t *testing.T) { + var released, collectionsReleased, foldersReleased, listedMicrosoft, listedVendor int + + fakes := []*fakeTask{ + {path: `\Microsoft\Task`, state: int32(TASK_STATE_READY)}, + {path: `\Vendor\Task`, state: int32(TASK_STATE_READY)}, + {path: `\Task`, state: int32(TASK_STATE_READY)}, + } + + folder := func(path string, task *fakeTask, listed *int) *ole.Object { + tasks, _ := fakeTaskCollection([]*ole.Object{task.object(&released)}, false, &collectionsReleased) + folders, _ := fakeTaskCollection(nil, false, &collectionsReleased) + + return &fakeTaskFolderAt(path, tasks, folders, &foldersReleased, listed).Object + } + + subfolders, _ := fakeTaskCollection([]*ole.Object{ + folder(`\Microsoft`, fakes[0], &listedMicrosoft), + folder(`\Vendor`, fakes[1], &listedVendor), + }, false, &collectionsReleased) + rootTasks, _ := fakeTaskCollection([]*ole.Object{fakes[2].object(&released)}, false, &collectionsReleased) + root := fakeTaskFolderAt(`\`, rootTasks, subfolders, nil, nil) + + c := New(tc.config) + require.NoError(t, c.Build(slog.New(slog.DiscardHandler), nil)) + + tasks := ScheduledTasks{} + + require.NoError(t, c.taskReader().fetchTasksRecursively(root, `\`, &tasks)) + + paths := make([]string, 0, len(tasks)) + for _, task := range tasks { + require.Equal(t, TASK_STATE_READY, task.State) + + paths = append(paths, task.Path) + } + + require.ElementsMatch(t, tc.paths, paths) + + // An excluded folder is released without listing its tasks or subfolders. + require.Equal(t, tc.listedMicrosoft, listedMicrosoft) + require.Equal(t, 2, listedVendor) + require.Equal(t, 2, foldersReleased) + require.Equal(t, tc.listedMicrosoft/2, fakes[0].reads[taskSlotPath]) + require.Equal(t, 1, fakes[1].reads[taskSlotState]) + }) + } +} + func TestFetchTasksFilters(t *testing.T) { for _, tc := range []struct { name string config *Config paths []string }{ - {name: "defaults", paths: []string{"/Included/Task", "/Included/Excluded", "/Other/Task"}}, + {name: "defaults", config: &Config{}, paths: []string{"/Included/Task", "/Included/Excluded", "/Other/Task"}}, { name: "include", config: &Config{TaskInclude: regexp.MustCompile(`^/Included/`)}, @@ -138,7 +203,9 @@ func TestFetchTasksFilters(t *testing.T) { collection, _ := fakeTaskCollection(items, false, &collectionsReleased) tasks := ScheduledTasks{} - err := fetchTasksInFolder(fakeTaskFolder(collection, nil, nil), New(tc.config).includeTask, &tasks) + tc.config.MissedRuns = true + + err := New(tc.config).taskReader().fetchTasksInFolder(fakeTaskFolder(collection, nil, nil), &tasks) require.NoError(t, err) require.Equal(t, len(fakes), released) @@ -155,7 +222,7 @@ func TestFetchTasksFilters(t *testing.T) { require.Equal(t, reads, task.reads[taskSlotMissedRuns], task.path) } - families := gatherTaskMetrics(t, nil, tasks) + families := gatherTaskMetrics(t, tc.config, tasks) if len(tc.paths) == 0 { require.Empty(t, families) @@ -210,7 +277,7 @@ func TestFetchTasksInFolderReadsPublishedProperties(t *testing.T) { ) tasks := ScheduledTasks{} - require.NoError(t, fetchTasksInFolder(fakeTaskFolder(collection, nil, nil), includeAllTasks, &tasks)) + require.NoError(t, readAllTasks.fetchTasksInFolder(fakeTaskFolder(collection, nil, nil), &tasks)) require.Equal(t, ScheduledTasks{ {Path: "/Folder/Ran", State: TASK_STATE_READY, LastTaskResult: 1, MissedRunsCount: 2}, {Path: "/NotRun", State: TASK_STATE_DISABLED, LastTaskResult: SCHED_S_TASK_HAS_NOT_RUN}, @@ -232,13 +299,32 @@ func TestFetchTasksInFolderReadsPublishedProperties(t *testing.T) { require.Equal(t, missedRuns, task.reads[taskSlotMissedRuns], task.path) } - missedRuns := gatherTaskMetrics(t, nil, tasks)["windows_scheduled_task_missed_runs"] + missedRuns := gatherTaskMetrics(t, &Config{MissedRuns: true}, tasks)["windows_scheduled_task_missed_runs"] require.NotNil(t, missedRuns) require.Len(t, missedRuns.GetMetric(), 1) require.Equal(t, "/Folder/Ran", missedRuns.GetMetric()[0].GetLabel()[0].GetValue()) require.InDelta(t, 2, missedRuns.GetMetric()[0].GetGauge().GetValue(), 0) } +// Missed runs are not read or published by default. +func TestFetchTasksInFolderSkipsMissedRuns(t *testing.T) { + var released, collectionsReleased int + + task := &fakeTask{path: `\Task`, state: int32(TASK_STATE_READY), result: 1, missed: 2} + collection, _ := fakeTaskCollection([]*ole.Object{task.object(&released)}, false, &collectionsReleased) + config := &Config{} + tasks := ScheduledTasks{} + + require.NoError(t, New(config).taskReader().fetchTasksInFolder(fakeTaskFolder(collection, nil, nil), &tasks)) + require.Equal(t, ScheduledTasks{{Path: "/Task", State: TASK_STATE_READY, LastTaskResult: 1}}, tasks) + require.Equal(t, 1, task.reads[taskSlotLastResult]) + require.Zero(t, task.reads[taskSlotMissedRuns]) + + families := gatherTaskMetrics(t, config, tasks) + require.Contains(t, families, "windows_scheduled_task_last_result") + require.NotContains(t, families, "windows_scheduled_task_missed_runs") +} + // The fake native collections use the Task Scheduler's one-based VARIANT indices. func fakeTaskCollection(items []*ole.Object, countError bool, released *int) (*ole.Object, *[]int64) { var methods [9]uintptr @@ -275,8 +361,20 @@ func fakeTaskCollection(items []*ole.Object, countError bool, released *int) (*o } func fakeTaskFolder(tasks, folders *ole.Object, released *int) *taskschd.TaskFolder { + return fakeTaskFolderAt("", tasks, folders, released, nil) +} + +// fakeTaskFolderAt returns a folder with the given path that counts the +// listings of its tasks and subfolders in listed when it is not nil. +func fakeTaskFolderAt(path string, tasks, folders *ole.Object, released, listed *int) *taskschd.TaskFolder { var methods [15]uintptr + count := func() { + if listed != nil { + *listed++ + } + } + methods[2] = windows.NewCallback(func(uintptr) uintptr { if released != nil { *released++ @@ -284,17 +382,17 @@ func fakeTaskFolder(tasks, folders *ole.Object, released *int) *taskschd.TaskFol return 0 }) - methods[8] = windows.NewCallback(func(_ uintptr, out *uintptr) uintptr { - *out = 0 // A null BSTR is a valid empty string. - - return 0 - }) + methods[8] = (&fakeTask{}).stringGetter(8, &path) // A null BSTR is a valid empty string. methods[10] = windows.NewCallback(func(_ uintptr, _ uintptr, out **ole.Object) uintptr { + count() + *out = folders return 0 }) methods[14] = windows.NewCallback(func(_ uintptr, _ uintptr, out **ole.Object) uintptr { + count() + *out = tasks return 0 @@ -387,11 +485,27 @@ var sysAllocString = windows.NewLazySystemDLL("oleaut32.dll").NewProc("SysAllocS func includeAllTasks(string) bool { return true } +//nolint:gochecknoglobals +var readAllTasks = taskReader{includeTask: includeAllTasks, includeFolder: includeAllTasks, missedRuns: true} + // BenchmarkGetScheduledTasks measures the Task Scheduler reads without metric creation. func BenchmarkGetScheduledTasks(b *testing.B) { - for b.Loop() { - if _, err := getScheduledTasks(includeAllTasks); err != nil { - b.Fatal(err) - } + for _, bc := range []struct { + name string + exclude string + }{ + {name: "all"}, + {name: "exclude Microsoft", exclude: "/Microsoft/.+"}, + } { + b.Run(bc.name, func(b *testing.B) { + c := New(&Config{TaskExclude: regexp.MustCompile(fmt.Sprintf("^(?:%s)$", bc.exclude))}) + require.NoError(b, c.Build(slog.New(slog.DiscardHandler), nil)) + + for b.Loop() { + if _, err := c.taskReader().getScheduledTasks(); err != nil { + b.Fatal(err) + } + } + }) } } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index dcb433ac..ed58ea76 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -489,3 +489,37 @@ func TestParse(t *testing.T) { }) } } + +// TestParseEmptyValue verifies that an empty value in the configuration file +// replaces a flag's non-empty default instead of keeping it. +func TestParseEmptyValue(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + config string + extraArgs []string + want string + }{ + {name: "default", config: "collector:\n scheduled_task:\n", want: "/Microsoft/.+"}, + {name: "empty string", config: "collector:\n scheduled_task:\n exclude: \"\"\n", want: ""}, + { + name: "CLI overrides config", config: "collector:\n scheduled_task:\n exclude: /Vendor/.+\n", + extraArgs: []string{"--collector.scheduled_task.exclude="}, want: "", + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "config.yaml") + require.NoError(t, os.WriteFile(path, []byte(tc.config), 0o600)) + + app := kingpin.New("test", "test") + app.Flag("config.file", "config").String() + collector.NewWithFlags(app) + + require.NoError(t, config.Parse(app, append([]string{"--config.file=" + path}, tc.extraArgs...))) + require.Equal(t, tc.want, app.GetFlag("collector.scheduled_task.exclude").Model().Value.String()) + }) + } +}