[prometheus.exporter.windows] Fix default values (#1878)

* Fix default values * Wrap regexes * Remove unnecessary comment * Fix comment * enabled_list in the smb block should be deprecated * Fix docs and comments * Apply suggestions from code review Co-authored-by: Clayton Cornell <[email protected]> * Replace "regex" with "regular expression" * Update changelog --------- Co-authored-by: Clayton Cornell <[email protected]>
grafana · Oct 17, 2024 · bbcfa11 · bbcfa11
1 parent b92ac52
commit bbcfa11
Show file tree

Hide file tree

Showing 9 changed files with 257 additions and 179 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -50,6 +50,8 @@ Main (unreleased)
 
 - Fixed a bug with `loki.source.podlogs` not starting in large clusters due to short informer sync timeout. (@elburnetto-intapp) 
 
+- `prometheus.exporter.windows`: Fixed bug with `exclude` regular expression config arguments which caused missing metrics. (@ptodev)
+
 ### Other changes
 
 - Small fix in UI stylesheet to fit more content into visible table area. (@defanator)

diff --git a/docs/sources/_index.md b/docs/sources/_index.md
@@ -6,6 +6,7 @@ weight: 350
 cascade:
   ALLOY_RELEASE: v1.5.0
   OTEL_VERSION: v0.105.0
+  PROM_WIN_EXP_VERSION: v0.27.3
   FULL_PRODUCT_NAME: Grafana Alloy
   PRODUCT_NAME: Alloy
 hero:

diff --git a/docs/sources/_index.md.t b/docs/sources/_index.md.t
@@ -6,6 +6,7 @@ weight: 350
 cascade:
   ALLOY_RELEASE: $ALLOY_VERSION
   OTEL_VERSION: v0.105.0
+  PROM_WIN_EXP_VERSION: v0.27.3
   FULL_PRODUCT_NAME: Grafana Alloy
   PRODUCT_NAME: Alloy
 hero:

diff --git a/docs/sources/reference/components/prometheus/prometheus.exporter.windows.md b/docs/sources/reference/components/prometheus/prometheus.exporter.windows.md
diff --git a/go.mod b/go.mod
@@ -149,7 +149,7 @@ require (
 	github.com/prometheus-community/elasticsearch_exporter v1.5.0
 	github.com/prometheus-community/postgres_exporter v0.11.1
 	github.com/prometheus-community/stackdriver_exporter v0.15.1
-	github.com/prometheus-community/windows_exporter v0.27.3
+	github.com/prometheus-community/windows_exporter v0.27.4-0.20241010144849-a0f6d3bcf9a4
 	github.com/prometheus-operator/prometheus-operator v0.66.0
 	github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.66.0
 	github.com/prometheus-operator/prometheus-operator/pkg/client v0.66.0

diff --git a/go.sum b/go.sum
@@ -2130,6 +2130,8 @@ github.com/prometheus-community/stackdriver_exporter v0.15.1 h1:+k26zeBy8BlG+eDK
 github.com/prometheus-community/stackdriver_exporter v0.15.1/go.mod h1:UmmIgnrVQqDAeM8pSeYntBcUxPhp8oqb8W3nvRYzsSg=
 github.com/prometheus-community/windows_exporter v0.27.3 h1:L5Dc4gqc3477Y6jaVHhkm25jysqbxg1ajMyPbmnqScw=
 github.com/prometheus-community/windows_exporter v0.27.3/go.mod h1:8+T6hfv71nvgVIzguouXkIGoa15ni+uXHHULBOA2bZo=
+github.com/prometheus-community/windows_exporter v0.27.4-0.20241010144849-a0f6d3bcf9a4 h1:e6RmefQvH1jXwo7JnN5UvcyZz8uyABMFScnkrrWNrf0=
+github.com/prometheus-community/windows_exporter v0.27.4-0.20241010144849-a0f6d3bcf9a4/go.mod h1:8+T6hfv71nvgVIzguouXkIGoa15ni+uXHHULBOA2bZo=
 github.com/prometheus-operator/prometheus-operator v0.66.0 h1:Jj4mbGAkfBbTih6ait03f2vUjEHB7Kb4gnlAmWu7AJ0=
 github.com/prometheus-operator/prometheus-operator v0.66.0/go.mod h1:U7S3+u6YTxwCTMNIQxZWttEq70qBA4Qps7/c5mUZOpQ=
 github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.66.0 h1:PPW01FLVjJHMNcbAL1DDD9EZceSQKMOU/VpK0irrxrI=

diff --git a/internal/component/prometheus/exporter/windows/config.go b/internal/component/prometheus/exporter/windows/config.go
@@ -1,11 +1,26 @@
 package windows
 
 import (
+	"fmt"
 	"strings"
 
 	windows_integration "github.com/grafana/alloy/internal/static/integrations/windows_exporter"
 )
 
+// Wrap some regex strings to prevent issues with user-supplied empty strings.
+// Prior to v0.27, the upstream exporter used to wrap regexes like this.
+// Alloy is now doing this instead, to maintain backwards compatibility.
+//
+// This is mostly to prevent issues with `exclude` arguments.
+// If `exclude` is set to `""` and there is no wrapping, the regex will match everything.
+// Therefore, all collectors will be excluded.
+//
+// See https://github.com/grafana/alloy/issues/1845
+// TODO: Remove this in Alloy v2.
+func wrapRegex(regex string) string {
+	return fmt.Sprintf("^(?:%s)$", regex)
+}
+
 // Arguments is used for controlling for this exporter.
 type Arguments struct {
 	// Collectors to mark as enabled
@@ -92,14 +107,14 @@ type IISConfig struct {
 // Convert converts the component's IISConfig to the integration's IISConfig.
 func (t IISConfig) Convert() windows_integration.IISConfig {
 	return windows_integration.IISConfig{
-		AppBlackList:  t.AppBlackList,
-		AppWhiteList:  t.AppWhiteList,
-		SiteBlackList: t.SiteBlackList,
-		SiteWhiteList: t.SiteWhiteList,
-		AppExclude:    t.AppExclude,
-		AppInclude:    t.AppInclude,
-		SiteExclude:   t.SiteExclude,
-		SiteInclude:   t.SiteInclude,
+		AppBlackList:  wrapRegex(t.AppBlackList),
+		AppWhiteList:  wrapRegex(t.AppWhiteList),
+		SiteBlackList: wrapRegex(t.SiteBlackList),
+		SiteWhiteList: wrapRegex(t.SiteWhiteList),
+		AppExclude:    wrapRegex(t.AppExclude),
+		AppInclude:    wrapRegex(t.AppInclude),
+		SiteExclude:   wrapRegex(t.SiteExclude),
+		SiteInclude:   wrapRegex(t.SiteInclude),
 	}
 }
 
@@ -126,10 +141,10 @@ type SMTPConfig struct {
 // Convert converts the component's SMTPConfig to the integration's SMTPConfig.
 func (t SMTPConfig) Convert() windows_integration.SMTPConfig {
 	return windows_integration.SMTPConfig{
-		BlackList: t.BlackList,
-		WhiteList: t.WhiteList,
-		Exclude:   t.Exclude,
-		Include:   t.Include,
+		BlackList: wrapRegex(t.BlackList),
+		WhiteList: wrapRegex(t.WhiteList),
+		Exclude:   wrapRegex(t.Exclude),
+		Include:   wrapRegex(t.Include),
 	}
 }
 
@@ -160,10 +175,10 @@ type ProcessConfig struct {
 // Convert converts the component's ProcessConfig to the integration's ProcessConfig.
 func (t ProcessConfig) Convert() windows_integration.ProcessConfig {
 	return windows_integration.ProcessConfig{
-		BlackList: t.BlackList,
-		WhiteList: t.WhiteList,
-		Exclude:   t.Exclude,
-		Include:   t.Include,
+		BlackList: wrapRegex(t.BlackList),
+		WhiteList: wrapRegex(t.WhiteList),
+		Exclude:   wrapRegex(t.Exclude),
+		Include:   wrapRegex(t.Include),
 	}
 }
 
@@ -176,8 +191,8 @@ type ScheduledTaskConfig struct {
 // Convert converts the component's ScheduledTaskConfig to the integration's ScheduledTaskConfig.
 func (t ScheduledTaskConfig) Convert() windows_integration.ScheduledTaskConfig {
 	return windows_integration.ScheduledTaskConfig{
-		Exclude: t.Exclude,
-		Include: t.Include,
+		Exclude: wrapRegex(t.Exclude),
+		Include: wrapRegex(t.Include),
 	}
 }
 
@@ -192,10 +207,10 @@ type NetworkConfig struct {
 // Convert converts the component's NetworkConfig to the integration's NetworkConfig.
 func (t NetworkConfig) Convert() windows_integration.NetworkConfig {
 	return windows_integration.NetworkConfig{
-		BlackList: t.BlackList,
-		WhiteList: t.WhiteList,
-		Exclude:   t.Exclude,
-		Include:   t.Include,
+		BlackList: wrapRegex(t.BlackList),
+		WhiteList: wrapRegex(t.WhiteList),
+		Exclude:   wrapRegex(t.Exclude),
+		Include:   wrapRegex(t.Include),
 	}
 }
 
@@ -234,10 +249,10 @@ type LogicalDiskConfig struct {
 // Convert converts the component's LogicalDiskConfig to the integration's LogicalDiskConfig.
 func (t LogicalDiskConfig) Convert() windows_integration.LogicalDiskConfig {
 	return windows_integration.LogicalDiskConfig{
-		BlackList: t.BlackList,
-		WhiteList: t.WhiteList,
-		Include:   t.Include,
-		Exclude:   t.Exclude,
+		BlackList: wrapRegex(t.BlackList),
+		WhiteList: wrapRegex(t.WhiteList),
+		Include:   wrapRegex(t.Include),
+		Exclude:   wrapRegex(t.Exclude),
 	}
 }
 
@@ -250,8 +265,8 @@ type PhysicalDiskConfig struct {
 // Convert converts the component's PhysicalDiskConfig to the integration's PhysicalDiskConfig.
 func (t PhysicalDiskConfig) Convert() windows_integration.PhysicalDiskConfig {
 	return windows_integration.PhysicalDiskConfig{
-		Include: t.Include,
-		Exclude: t.Exclude,
+		Include: wrapRegex(t.Include),
+		Exclude: wrapRegex(t.Exclude),
 	}
 }
 
@@ -264,8 +279,8 @@ type PrinterConfig struct {
 // Convert converts the component's ProcessConfig to the integration's ProcessConfig.
 func (t PrinterConfig) Convert() windows_integration.PrinterConfig {
 	return windows_integration.PrinterConfig{
-		Exclude: t.Exclude,
-		Include: t.Include,
+		Exclude: wrapRegex(t.Exclude),
+		Include: wrapRegex(t.Include),
 	}
 }
 

diff --git a/internal/component/prometheus/exporter/windows/config_default_windows_test.go b/internal/component/prometheus/exporter/windows/config_default_windows_test.go
@@ -1,6 +1,8 @@
 package windows
 
 import (
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/grafana/alloy/syntax"
@@ -44,3 +46,43 @@ func TestAlloyUnmarshalWithDefaultConfig(t *testing.T) {
 	require.Equal(t, defaultArgs.SMTP.Include, args.SMTP.Include)
 	require.Equal(t, defaultArgs.TextFile.TextFileDirectory, args.TextFile.TextFileDirectory)
 }
+
+// This is a copy of the getDefaultPath() function in:
+// [email protected]\pkg\collector\textfile\textfile.go
+func getDefaultTextFilePath() string {
+	execPath, _ := os.Executable()
+	return filepath.Join(filepath.Dir(execPath), "textfile_inputs")
+}
+
+func TestDefaultConfig(t *testing.T) {
+	// TODO: The BlackList and WhiteList attributes should be removed in Alloy v2.
+	// They are not even documented in Alloy v1.
+	expected := Arguments{
+		EnabledCollectors: []string{"cpu", "cs", "logical_disk", "net", "os", "service", "system"},
+		Dfsr:              DfsrConfig{SourcesEnabled: []string{"connection", "folder", "volume"}},
+		Exchange:          ExchangeConfig{EnabledList: []string{"ADAccessProcesses", "TransportQueues", "HttpProxy", "ActiveSync", "AvailabilityService", "OutlookWebAccess", "Autodiscover", "WorkloadManagement", "RpcClientAccess", "MapiHttpEmsmdb"}},
+		IIS:               IISConfig{AppBlackList: "^$", AppWhiteList: "^.+$", SiteBlackList: "^$", SiteWhiteList: "^.+$", AppExclude: "^$", AppInclude: "^.+$", SiteExclude: "^$", SiteInclude: "^.+$"},
+		LogicalDisk:       LogicalDiskConfig{BlackList: "^$", WhiteList: "^.+$", Include: "^.+$", Exclude: "^$"},
+		MSMQ:              MSMQConfig{Where: ""},
+		MSSQL:             MSSQLConfig{EnabledClasses: []string{"accessmethods", "availreplica", "bufman", "databases", "dbreplica", "genstats", "locks", "memmgr", "sqlstats", "sqlerrors", "transactions", "waitstats"}},
+		Network:           NetworkConfig{BlackList: "^$", WhiteList: "^.+$", Exclude: "^$", Include: "^.+$"},
+		PhysicalDisk:      PhysicalDiskConfig{Include: "^.+$", Exclude: "^$"},
+		Printer:           PrinterConfig{Exclude: "^$", Include: "^.+$"},
+		Process:           ProcessConfig{BlackList: "^$", WhiteList: "^.+$", Exclude: "^$", Include: "^.+$"},
+		ScheduledTask:     ScheduledTaskConfig{Exclude: "^$", Include: "^.+$"},
+		Service:           ServiceConfig{UseApi: "false", Where: "", V2: "false"},
+		SMB:               SMBConfig{EnabledList: []string{}},
+		SMBClient:         SMBClientConfig{EnabledList: []string{}},
+		SMTP:              SMTPConfig{BlackList: "^$", WhiteList: "^.+$", Exclude: "^$", Include: "^.+$"},
+		TextFile:          TextFileConfig{TextFileDirectory: getDefaultTextFilePath()},
+	}
+
+	var args Arguments
+	err := syntax.Unmarshal([]byte(""), &args)
+	require.NoError(t, err)
+	require.Equal(t, expected, args)
+
+	var defaultArgs Arguments
+	defaultArgs.SetToDefault()
+	require.Equal(t, expected, defaultArgs)
+}
diff --git a/internal/component/prometheus/exporter/windows/windows_test.go b/internal/component/prometheus/exporter/windows/windows_test.go
@@ -119,27 +119,27 @@ func TestConvert(t *testing.T) {
 
 	require.Equal(t, "textfile,cpu", conf.EnabledCollectors)
 	require.Equal(t, "example", conf.Exchange.EnabledList)
-	require.Equal(t, "", conf.IIS.SiteExclude)
-	require.Equal(t, ".+", conf.IIS.SiteInclude)
-	require.Equal(t, "", conf.IIS.AppExclude)
-	require.Equal(t, ".+", conf.IIS.AppInclude)
+	require.Equal(t, "^(?:)$", conf.IIS.SiteExclude)
+	require.Equal(t, "^(?:.+)$", conf.IIS.SiteInclude)
+	require.Equal(t, "^(?:)$", conf.IIS.AppExclude)
+	require.Equal(t, "^(?:.+)$", conf.IIS.AppInclude)
 	require.Equal(t, "C:", conf.TextFile.TextFileDirectory)
-	require.Equal(t, "", conf.SMTP.Exclude)
-	require.Equal(t, ".+", conf.SMTP.Include)
+	require.Equal(t, "^(?:)$", conf.SMTP.Exclude)
+	require.Equal(t, "^(?:.+)$", conf.SMTP.Include)
 	require.Equal(t, "where", conf.Service.Where)
 	require.Equal(t, "true", conf.Service.V2)
-	require.Equal(t, "", conf.PhysicalDisk.Exclude)
-	require.Equal(t, ".+", conf.PhysicalDisk.Include)
-	require.Equal(t, "", conf.Process.Exclude)
-	require.Equal(t, ".+", conf.Process.Include)
-	require.Equal(t, "", conf.Printer.Exclude)
-	require.Equal(t, ".+", conf.Printer.Include)
+	require.Equal(t, "^(?:)$", conf.PhysicalDisk.Exclude)
+	require.Equal(t, "^(?:.+)$", conf.PhysicalDisk.Include)
+	require.Equal(t, "^(?:)$", conf.Process.Exclude)
+	require.Equal(t, "^(?:.+)$", conf.Process.Include)
+	require.Equal(t, "^(?:)$", conf.Printer.Exclude)
+	require.Equal(t, "^(?:.+)$", conf.Printer.Include)
 	require.Equal(t, "example", conf.SMB.EnabledList)
 	require.Equal(t, "example", conf.SMBClient.EnabledList)
-	require.Equal(t, "", conf.Network.Exclude)
-	require.Equal(t, ".+", conf.Network.Include)
+	require.Equal(t, "^(?:)$", conf.Network.Exclude)
+	require.Equal(t, "^(?:.+)$", conf.Network.Include)
 	require.Equal(t, "accessmethods", conf.MSSQL.EnabledClasses)
 	require.Equal(t, "where", conf.MSMQ.Where)
-	require.Equal(t, "", conf.LogicalDisk.Exclude)
-	require.Equal(t, ".+", conf.LogicalDisk.Include)
+	require.Equal(t, "^(?:)$", conf.LogicalDisk.Exclude)
+	require.Equal(t, "^(?:.+)$", conf.LogicalDisk.Include)
 }