log: open the event log lazily, bound close, keep the service running, structured messages (#2734)

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

cmd/windows_exporter/0_service.go

@@ -21,10 +21,13 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"io"
 	"log/slog"
 	"os"
+	"path/filepath"
 	"strings"
 	"sync"
+	"time"
 	"unsafe"
 
 	"golang.org/x/sys/windows"
@@ -96,10 +99,11 @@ var IsService = func() bool {
 		}()
 	}()
 
+	// This event is informational. If it cannot be written, the exporter still
+	// starts: stopping the service here would report it as stopped while run
+	// keeps serving.
 	if err := logToEventToLog(windows.EVENTLOG_INFORMATION_TYPE, "attempting to start exporter service"); err != nil {
 		logToFile(fmt.Sprintf("failed sent log to event log: %v", err))
-
-		exitCodeCh <- 2
 	}
 
 	return true
@@ -222,11 +226,74 @@ func logStartupError(ctx context.Context, msg string, err error) {
 	)
 }
 
+const (
+	// serviceErrorLogName is the file in the temporary directory that logToFile appends to.
+	serviceErrorLogName = "windows_exporter.service.error.log"
+	// serviceErrorLogMaxSize is the size above which logToFile starts the file over.
+	serviceErrorLogMaxSize = 1 << 20
+)
+
+// logToFile is the last resort when the event log cannot be written. It appends
+// msg to one file in the temporary directory, so that repeated failures don't
+// leave a new file behind each time. For a service running as LocalSystem, Go
+// returns C:\Windows\SystemTemp, which only administrators can write to.
 func logToFile(msg string) {
-	if file, err := os.CreateTemp("", "windows_exporter.service.error.log"); err == nil {
-		_, _ = file.WriteString(msg)
-		_ = file.Close()
+	_ = appendToFile(filepath.Join(os.TempDir(), serviceErrorLogName), msg, time.Now(), serviceErrorLogMaxSize)
+}
+
+// appendToFile appends a timestamped line to path. It starts the file over once
+// it is larger than maxSize. It refuses to write through a symbolic link,
+// junction or other reparse point, or to a file with more than one hard link,
+// so that a file planted at path cannot redirect the write to another file.
+func appendToFile(path, msg string, now time.Time, maxSize int64) error {
+	pathPtr, err := windows.UTF16PtrFromString(path)
+	if err != nil {
+		return err
 	}
+
+	handle, err := windows.CreateFile(
+		pathPtr,
+		windows.GENERIC_READ|windows.GENERIC_WRITE,
+		windows.FILE_SHARE_READ,
+		nil,
+		windows.OPEN_ALWAYS,
+		windows.FILE_ATTRIBUTE_NORMAL|windows.FILE_FLAG_OPEN_REPARSE_POINT,
+		0,
+	)
+	if err != nil {
+		return fmt.Errorf("open %s: %w", path, err)
+	}
+
+	file := os.NewFile(uintptr(handle), path)
+
+	defer func() {
+		_ = file.Close()
+	}()
+
+	var info windows.ByHandleFileInformation
+	if err = windows.GetFileInformationByHandle(handle, &info); err != nil {
+		return fmt.Errorf("stat %s: %w", path, err)
+	}
+
+	if info.FileAttributes&windows.FILE_ATTRIBUTE_REPARSE_POINT != 0 || info.NumberOfLinks > 1 {
+		return fmt.Errorf("refusing to write to %s: it is a reparse point or has more than one link", path)
+	}
+
+	if size := int64(info.FileSizeHigh)<<32 | int64(info.FileSizeLow); size > maxSize {
+		if err = file.Truncate(0); err != nil {
+			return fmt.Errorf("truncate %s: %w", path, err)
+		}
+	}
+
+	if _, err = file.Seek(0, io.SeekEnd); err != nil {
+		return fmt.Errorf("seek %s: %w", path, err)
+	}
+
+	if _, err = file.WriteString(now.Format(time.RFC3339) + " " + strings.TrimSpace(msg) + "\n"); err != nil {
+		return fmt.Errorf("write %s: %w", path, err)
+	}
+
+	return nil
 }
 
 // isWindowsService is a clone of "golang.org/x/sys/windows/svc:IsWindowsService", but with a fix

cmd/windows_exporter/main.go

@@ -200,7 +200,9 @@ func run(ctx context.Context, args []string) int {
 	logger.LogAttrs(ctx, slog.LevelDebug, "logging has Started")
 
 	if configFile != nil && *configFile != "" {
-		logger.LogAttrs(ctx, slog.LevelInfo, "using configuration file: "+*configFile)
+		logger.LogAttrs(ctx, slog.LevelInfo, "using configuration file",
+			slog.String("file", *configFile),
+		)
 	}
 
 	// Log the build and the account before the collectors are built,
@@ -261,7 +263,9 @@ func run(ctx context.Context, args []string) int {
 		return 1
 	}
 
-	logger.InfoContext(ctx, "Enabled collectors: "+strings.Join(effectiveCollectors(enabledCollectorList, disabledCollectorList), ", "))
+	logger.LogAttrs(ctx, slog.LevelInfo, "enabled collectors",
+		slog.String("collectors", strings.Join(effectiveCollectors(enabledCollectorList, disabledCollectorList), ",")),
+	)
 
 	handlerOpts := &httphandler.Options{
 		DisableExporterMetrics: *disableExporterMetrics,
@@ -284,7 +288,9 @@ func run(ctx context.Context, args []string) int {
 		mux.HandleFunc("GET /debug/pprof/trace", pprof.Trace)
 	}
 
-	logger.LogAttrs(ctx, slog.LevelInfo, fmt.Sprintf("started windows_exporter in %s", time.Since(startTime)))
+	logger.LogAttrs(ctx, slog.LevelInfo, "started windows_exporter",
+		slog.Duration("duration", time.Since(startTime)),
+	)
 
 	// Requests derive their context from serveCtx, so that canceling it cancels in-flight scrapes.
 	serveCtx, cancelServe := context.WithCancel(ctx)
@@ -384,7 +390,9 @@ func logCurrentUser(ctx context.Context, logger *slog.Logger) {
 		return
 	}
 
-	logger.LogAttrs(ctx, slog.LevelInfo, "Running as "+u.Username)
+	logger.LogAttrs(ctx, slog.LevelInfo, "running as user",
+		slog.String("user", u.Username),
+	)
 
 	if strings.Contains(u.Username, "ContainerAdministrator") || strings.Contains(u.Username, "ContainerUser") {
 		logger.LogAttrs(ctx, slog.LevelWarn, "Running as a preconfigured Windows Container user. This may mean you do not have Windows HostProcess containers configured correctly and some functionality will not work as expected.")
@@ -460,7 +468,9 @@ func closeCollection(ctx context.Context, logger *slog.Logger, collection *colle
 			)
 		}
 	case <-timer.C:
-		logger.LogAttrs(ctx, slog.LevelWarn, fmt.Sprintf("collectors didn't close within %s", collectionCloseTimeout))
+		logger.LogAttrs(ctx, slog.LevelWarn, "collectors didn't close in time",
+			slog.Duration("timeout", collectionCloseTimeout),
+		)
 	}
 }
 

cmd/windows_exporter/service_test.go

@@ -19,6 +19,8 @@ package main
 
 import (
 	"context"
+	"os"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -26,6 +28,62 @@ import (
 	"golang.org/x/sys/windows/svc"
 )
 
+func TestAppendToFile(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	path := filepath.Join(dir, serviceErrorLogName)
+	now := time.Date(2026, 10, 15, 14, 3, 5, 0, time.UTC)
+
+	require.NoError(t, appendToFile(path, "first failure", now, serviceErrorLogMaxSize))
+	require.NoError(t, appendToFile(path, "second failure\n", now.Add(time.Second), serviceErrorLogMaxSize))
+
+	entries, err := os.ReadDir(dir)
+	require.NoError(t, err)
+	require.Len(t, entries, 1, "repeated failures must not create a new file each time")
+
+	data, err := os.ReadFile(path)
+	require.NoError(t, err)
+	require.Equal(t, "2026-10-15T14:03:05Z first failure\n2026-10-15T14:03:06Z second failure\n", string(data))
+}
+
+func TestAppendToFileStartsOverAboveMaxSize(t *testing.T) {
+	t.Parallel()
+
+	path := filepath.Join(t.TempDir(), serviceErrorLogName)
+	now := time.Date(2026, 10, 15, 14, 3, 5, 0, time.UTC)
+
+	require.NoError(t, appendToFile(path, "first failure", now, 10))
+	require.NoError(t, appendToFile(path, "second failure", now, 10))
+
+	data, err := os.ReadFile(path)
+	require.NoError(t, err)
+	require.Equal(t, "2026-10-15T14:03:05Z second failure\n", string(data))
+}
+
+func TestAppendToFileRefusesLinks(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	target := filepath.Join(dir, "target.txt")
+	require.NoError(t, os.WriteFile(target, []byte("keep"), 0o600))
+
+	hardLink := filepath.Join(dir, "hardlink.log")
+	require.NoError(t, os.Link(target, hardLink))
+	require.ErrorContains(t, appendToFile(hardLink, "failure", time.Now(), serviceErrorLogMaxSize), "refusing to write")
+
+	symlink := filepath.Join(dir, "symlink.log")
+	if err := os.Symlink(target, symlink); err == nil {
+		require.ErrorContains(t, appendToFile(symlink, "failure", time.Now(), serviceErrorLogMaxSize), "refusing to write")
+	} else {
+		t.Logf("skipping the symbolic link case: %v", err)
+	}
+
+	data, err := os.ReadFile(target)
+	require.NoError(t, err)
+	require.Equal(t, "keep", string(data))
+}
+
 func TestServiceExecuteStop(t *testing.T) {
 	t.Parallel()