fix: reject unknown process priorities and trap SIGTERM instead of os.Kill (#2548)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

README.md

@@ -89,7 +89,7 @@ windows_exporter accepts flags to configure certain behaviours. The ones configu
 | `--web.listen-address`    | host:port for exporter.                                                                                                                                                                          | `:9182`       |
 | `--telemetry.path`        | URL path for surfacing collected metrics.                                                                                                                                                        | `/metrics`    |
 | `--collectors.enabled`    | Comma-separated list of collectors to use. Use `[defaults]` as a placeholder which gets expanded containing all the collectors enabled by default.                                               | `[defaults]`  |
-| `--scrape.timeout-margin` | Seconds to subtract from the timeout allowed by the client. Tune to allow for overhead or high loads.                                                                                            | `0.5`         |
+| `--scrape.timeout-margin` | Seconds to subtract from the timeout allowed by the client. The margin takes at most half of the client's timeout. Tune to allow for overhead or high loads.                                     | `0.5`         |
 | `--web.config.file`       | A [web config][web_config] for setting up TLS and Auth                                                                                                                                           | None          |
 | `--config.file`           | [Using a config file](#using-a-configuration-file) from path                                                                                                                                     | None          |
 | `--log.file`              | Output file of log messages. One of [stdout, stderr, eventlog, \<path to log file>]<br>**NOTE:** The MSI installer will add a default argument to the installed service setting this to eventlog | stderr        |

cmd/windows_exporter/main.go

@@ -24,6 +24,7 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"maps"
 	"net/http"
 	"net/http/pprof"
 	"os"
@@ -33,6 +34,7 @@ import (
 	"runtime/debug"
 	"slices"
 	"strings"
+	"syscall"
 	"time"
 
 	"github.com/alecthomas/kingpin/v2"
@@ -52,7 +54,9 @@ import (
 const collectionCloseTimeout = 10 * time.Second
 
 func main() {
-	ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, os.Kill)
+	// os.Kill can't be caught. On Windows, Go delivers console close, logoff and shutdown events as SIGTERM.
+	//nolint:forbidigo // os/signal only accepts syscall.Signal values, not windows.Signal.
+	ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
 
 	// A stop request from the service manager cancels the root context,
 	// so that it aborts the startup as well as the running exporter.
@@ -111,7 +115,7 @@ func run(ctx context.Context, args []string) int {
 			Default("").String()
 		timeoutMargin = app.Flag(
 			"scrape.timeout-margin",
-			"Seconds to subtract from the timeout allowed by the client. Tune to allow for overhead or high loads.",
+			"Seconds to subtract from the timeout allowed by the client. The margin takes at most half of the client's timeout. Tune to allow for overhead or high loads.",
 		).Default("0.5").Float64()
 		debugEnabled = app.Flag(
 			"debug.enabled",
@@ -324,9 +328,12 @@ func setPriorityWindows(ctx context.Context, logger *slog.Logger, pid int, prior
 	}
 
 	winPriority, ok := priorityStringToInt[priority]
+	if !ok {
+		return fmt.Errorf("unknown process priority %q, must be one of %s", priority, strings.Join(slices.Sorted(maps.Keys(priorityStringToInt)), ", "))
+	}
 
-	// Only set process priority if a non-default and valid value has been set
-	if !ok || winPriority == windows.NORMAL_PRIORITY_CLASS {
+	// Only set process priority if a non-default value has been set
+	if winPriority == windows.NORMAL_PRIORITY_CLASS {
 		return nil
 	}
 

cmd/windows_exporter/main_test.go

@@ -22,6 +22,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"log/slog"
 	"net"
 	"net/http"
 	"net/url"
@@ -175,3 +176,12 @@ func waitUntilListening(tb testing.TB, network, address string) error {
 
 	return fmt.Errorf("listener not listening: %w", err)
 }
+
+func TestSetPriorityWindows(t *testing.T) {
+	t.Parallel()
+
+	logger := slog.New(slog.DiscardHandler)
+
+	require.NoError(t, setPriorityWindows(t.Context(), logger, os.Getpid(), "normal"))
+	require.ErrorContains(t, setPriorityWindows(t.Context(), logger, os.Getpid(), "highest"), `unknown process priority "highest"`)
+}