Move smart agent config validation from unmarshal

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### 🧰 Bug fixes 🧰
+
+- (Splunk) `receiver/smartagent`: Fix the receiver failing to start by receiver_creator since 0.124.0 ([#6187](https://github.com/signalfx/splunk-otel-collector/pull/6187))
+
 ## v0.124.0
 
 This Splunk OpenTelemetry Collector release includes changes from the [opentelemetry-collector v0.124.0](https://github.com/open-telemetry/opentelemetry-collector/releases/tag/v0.124.0)

pkg/receiver/smartagentreceiver/config.go

@@ -48,17 +48,19 @@ var (
 )
 
 type Config struct {
-	monitorConfig saconfig.MonitorCustomConfig
+	MonitorType string `mapstructure:"type"` // Smart Agent monitor type, e.g. collectd/cpu
 	// Generally an observer/receivercreator-set value via Endpoint.Target.
 	// Will expand to MonitorCustomConfig Host and Port values if unset.
 	Endpoint         string   `mapstructure:"endpoint"`
 	DimensionClients []string `mapstructure:"dimensionClients"`
+
+	monitorConfig    saconfig.MonitorCustomConfig
 	acceptsEndpoints bool
 }
 
 func (cfg *Config) validate() error {
-	if cfg == nil || cfg.monitorConfig == nil {
-		return fmt.Errorf("you must supply a valid Smart Agent Monitor config")
+	if cfg.MonitorType == "" {
+		return fmt.Errorf(`you must specify a "type" for a smartagent receiver`)
 	}
 
 	monitorConfigCore := cfg.monitorConfig.MonitorConfigCore()
@@ -77,40 +79,33 @@ func (cfg *Config) validate() error {
 // Unmarshal dynamically creates the desired Smart Agent monitor config
 // from the provided receiver config content.
 func (cfg *Config) Unmarshal(componentParser *confmap.Conf) error {
-	// AllSettings() is the user provided config and intoCfg is the default Config instance we populate to
-	// form the final desired version. To do so, we manually obtain all Config items, leaving only Smart Agent
-	// monitor config settings to be unmarshalled to their respective custom monitor config types.
-	allSettings := componentParser.ToStringMap()
-	monitorType, ok := allSettings["type"].(string)
-	if !ok || monitorType == "" {
-		return fmt.Errorf("you must specify a \"type\" for a smartagent receiver")
+	// Load the non-dynamic config normally ignoring unused fields.
+	if err := componentParser.Unmarshal(cfg, confmap.WithIgnoreUnused()); err != nil {
+		return err
 	}
 
-	var endpoint any
-	if endpoint, ok = allSettings["endpoint"]; ok {
-		cfg.Endpoint = fmt.Sprintf("%s", endpoint)
-		delete(allSettings, "endpoint")
+	// No need to proceed if monitor type isn't specified, this will fail the validation.
+	if cfg.MonitorType == "" {
+		return nil
 	}
 
-	var err error
-	cfg.DimensionClients, err = getStringSliceFromAllSettings(allSettings, "dimensionClients", errDimensionClientValue)
-	if err != nil {
-		return err
-	}
+	monitorConfmap := componentParser.ToStringMap()
+	delete(monitorConfmap, "endpoint")
+	delete(monitorConfmap, "dimensionClients")
 
 	// monitors.ConfigTemplates is a map that all monitors use to register their custom configs in the Smart Agent.
 	// The values are always pointers to an actual custom config.
-	var customMonitorConfig saconfig.MonitorCustomConfig
-	if customMonitorConfig, ok = monitors.ConfigTemplates[monitorType]; !ok {
-		if unsupported := nonWindowsMonitors[monitorType]; runtime.GOOS == "windows" && unsupported {
-			return fmt.Errorf("smart agent monitor type %q is not supported on windows platforms", monitorType)
+	customMonitorConfig, ok := monitors.ConfigTemplates[cfg.MonitorType]
+	if !ok {
+		if unsupported := nonWindowsMonitors[cfg.MonitorType]; runtime.GOOS == "windows" && unsupported {
+			return fmt.Errorf("smart agent monitor type %q is not supported on windows platforms", cfg.MonitorType)
 		}
-		return fmt.Errorf("no known monitor type %q", monitorType)
+		return fmt.Errorf("no known monitor type %q", cfg.MonitorType)
 	}
 	monitorConfigType := reflect.TypeOf(customMonitorConfig).Elem()
 	monitorConfig := reflect.New(monitorConfigType).Interface()
 
-	asBytes, err := yaml.Marshal(allSettings)
+	asBytes, err := yaml.Marshal(monitorConfmap)
 	if err != nil {
 		return fmt.Errorf("failed constructing raw Smart Agent Monitor config block: %w", err)
 	}
@@ -140,26 +135,6 @@ func (cfg *Config) Unmarshal(componentParser *confmap.Conf) error {
 	return nil
 }
 
-func getStringSliceFromAllSettings(allSettings map[string]any, key string, errToReturn error) ([]string, error) {
-	var items []string
-	if value, ok := allSettings[key]; ok {
-		items = []string{}
-		if valueAsSlice, isSlice := value.([]any); isSlice {
-			for _, c := range valueAsSlice {
-				if client, isString := c.(string); isString {
-					items = append(items, client)
-				} else {
-					return nil, errToReturn
-				}
-			}
-		} else {
-			return nil, errToReturn
-		}
-		delete(allSettings, key)
-	}
-	return items, nil
-}
-
 // If using the receivercreator, observer-provided endpoints should be used to set
 // the Host and Port fields of monitor config structs.  This can only be done by reflection without
 // making type assertions over all possible monitor types.

pkg/receiver/smartagentreceiver/config_linux_test.go

@@ -44,6 +44,7 @@ func TestLoadConfigWithLinuxOnlyMonitors(t *testing.T) {
 	err = cm.Unmarshal(&apacheCfg)
 	require.NoError(t, err)
 	require.Equal(t, &Config{
+		MonitorType: "collectd/apache",
 		monitorConfig: &apache.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "collectd/apache",
@@ -64,6 +65,7 @@ func TestLoadConfigWithLinuxOnlyMonitors(t *testing.T) {
 	err = cm.Unmarshal(&kafkaCfg)
 	require.NoError(t, err)
 	require.Equal(t, &Config{
+		MonitorType: "collectd/kafka",
 		monitorConfig: &kafka.Config{
 			Config: genericjmx.Config{
 				MonitorConfig: saconfig.MonitorConfig{
@@ -87,6 +89,7 @@ func TestLoadConfigWithLinuxOnlyMonitors(t *testing.T) {
 	err = cm.Unmarshal(&memcachedCfg)
 	require.NoError(t, err)
 	require.Equal(t, &Config{
+		MonitorType: "collectd/memcached",
 		monitorConfig: &memcached.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "collectd/memcached",
@@ -106,6 +109,7 @@ func TestLoadConfigWithLinuxOnlyMonitors(t *testing.T) {
 	err = cm.Unmarshal(&phpCfg)
 	require.NoError(t, err)
 	require.Equal(t, &Config{
+		MonitorType: "collectd/php-fpm",
 		monitorConfig: &php.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "collectd/php-fpm",

pkg/receiver/smartagentreceiver/config_test.go

@@ -60,6 +60,7 @@ func TestLoadConfig(t *testing.T) {
 
 	expectedDimensionClients := []string{"nop/one", "nop/two"}
 	require.Equal(t, &Config{
+		MonitorType:      "haproxy",
 		DimensionClients: expectedDimensionClients,
 		monitorConfig: &haproxy.Config{
 			MonitorConfig: saconfig.MonitorConfig{
@@ -83,6 +84,7 @@ func TestLoadConfig(t *testing.T) {
 	require.NoError(t, cm.Unmarshal(&redisCfg))
 
 	require.Equal(t, &Config{
+		MonitorType:      "collectd/redis",
 		DimensionClients: []string{},
 		monitorConfig: &redis.Config{
 			MonitorConfig: saconfig.MonitorConfig{
@@ -103,6 +105,7 @@ func TestLoadConfig(t *testing.T) {
 	require.NoError(t, cm.Unmarshal(&hadoopCfg))
 
 	require.Equal(t, &Config{
+		MonitorType: "collectd/hadoop",
 		monitorConfig: &hadoop.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "collectd/hadoop",
@@ -123,6 +126,7 @@ func TestLoadConfig(t *testing.T) {
 	require.NoError(t, cm.Unmarshal(&etcdCfg))
 
 	require.Equal(t, &Config{
+		MonitorType: "etcd",
 		monitorConfig: &prometheusexporter.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "etcd",
@@ -147,6 +151,7 @@ func TestLoadConfig(t *testing.T) {
 	ntpqCfg := CreateDefaultConfig().(*Config)
 	require.NoError(t, cm.Unmarshal(&ntpqCfg))
 	require.Equal(t, &Config{
+		MonitorType: "telegraf/ntpq",
 		monitorConfig: &ntpq.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "telegraf/ntpq",
@@ -167,6 +172,8 @@ func TestLoadInvalidConfigWithoutType(t *testing.T) {
 	require.NoError(t, err)
 	withoutType := CreateDefaultConfig().(*Config)
 	err = cm.Unmarshal(&withoutType)
+	require.NoError(t, err)
+	err = withoutType.validate()
 	require.Error(t, err)
 	require.ErrorContains(t, err,
 		`you must specify a "type" for a smartagent receiver`)
@@ -208,6 +215,7 @@ func TestLoadInvalidConfigs(t *testing.T) {
 	negativeIntervalCfg := CreateDefaultConfig().(*Config)
 	require.NoError(t, cm.Unmarshal(&negativeIntervalCfg))
 	require.Equal(t, &Config{
+		MonitorType: "collectd/redis",
 		monitorConfig: &redis.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "collectd/redis",
@@ -226,6 +234,7 @@ func TestLoadInvalidConfigs(t *testing.T) {
 	missingRequiredCfg := CreateDefaultConfig().(*Config)
 	require.NoError(t, cm.Unmarshal(&missingRequiredCfg))
 	require.Equal(t, &Config{
+		MonitorType: "collectd/consul",
 		monitorConfig: &consul.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "collectd/consul",
@@ -256,7 +265,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
 	haproxyCfg := CreateDefaultConfig().(*Config)
 	require.NoError(t, cm.Unmarshal(&haproxyCfg))
 	require.Equal(t, &Config{
-		Endpoint: "[fe80::20c:29ff:fe59:9446]:2345",
+		MonitorType: "haproxy",
+		Endpoint:    "[fe80::20c:29ff:fe59:9446]:2345",
 		monitorConfig: &haproxy.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "haproxy",
@@ -280,7 +290,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
 	redisCfg := CreateDefaultConfig().(*Config)
 	require.NoError(t, cm.Unmarshal(&redisCfg))
 	require.Equal(t, &Config{
-		Endpoint: "redishost",
+		MonitorType: "collectd/redis",
+		Endpoint:    "redishost",
 		monitorConfig: &redis.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "collectd/redis",
@@ -299,7 +310,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
 	hadoopCfg := CreateDefaultConfig().(*Config)
 	require.NoError(t, cm.Unmarshal(&hadoopCfg))
 	require.Equal(t, &Config{
-		Endpoint: "[::]:12345",
+		MonitorType: "collectd/hadoop",
+		Endpoint:    "[::]:12345",
 		monitorConfig: &hadoop.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "collectd/hadoop",
@@ -319,7 +331,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
 	etcdCfg := CreateDefaultConfig().(*Config)
 	require.NoError(t, cm.Unmarshal(&etcdCfg))
 	require.Equal(t, &Config{
-		Endpoint: "etcdhost:5555",
+		MonitorType: "etcd",
+		Endpoint:    "etcdhost:5555",
 		monitorConfig: &prometheusexporter.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "etcd",
@@ -344,7 +357,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
 	require.NoError(t, cm.Unmarshal(&elasticCfg))
 	tru := true
 	require.Equal(t, &Config{
-		Endpoint: "elastic:567",
+		MonitorType: "elasticsearch",
+		Endpoint:    "elastic:567",
 		monitorConfig: &stats.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "elasticsearch",
@@ -373,7 +387,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
 	kubeletCfg := CreateDefaultConfig().(*Config)
 	require.NoError(t, cm.Unmarshal(&kubeletCfg))
 	require.Equal(t, &Config{
-		Endpoint: "disregarded:678",
+		MonitorType: "kubelet-stats",
+		Endpoint:    "disregarded:678",
 		monitorConfig: &cadvisor.KubeletStatsConfig{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "kubelet-stats",
@@ -415,7 +430,8 @@ func TestLoadConfigWithUnsupportedEndpoint(t *testing.T) {
 	require.NoError(t, cm.Unmarshal(&nagiosCfg))
 
 	require.Equal(t, &Config{
-		Endpoint: "localhost:12345",
+		MonitorType: "nagios",
+		Endpoint:    "localhost:12345",
 		monitorConfig: &nagios.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "nagios",
@@ -437,9 +453,24 @@ func TestLoadInvalidConfigWithNonArrayDimensionClients(t *testing.T) {
 	require.NoError(t, err)
 	haproxyCfg := CreateDefaultConfig().(*Config)
 	err = cm.Unmarshal(&haproxyCfg)
-	require.Error(t, err)
-	require.ErrorContains(t, err,
-		`dimensionClients must be an array of compatible exporter names`)
+	require.NoError(t, err)
+	require.Equal(t, &Config{
+		MonitorType:      "haproxy",
+		DimensionClients: []string{"notanarray"},
+		monitorConfig: &haproxy.Config{
+			MonitorConfig: saconfig.MonitorConfig{
+				Type:                "haproxy",
+				DatapointsToExclude: []saconfig.MetricFilter{},
+				IntervalSeconds:     123,
+			},
+			Username:  "SomeUser",
+			Password:  "secret",
+			Path:      "stats?stats;csv",
+			SSLVerify: true,
+			Timeout:   5000000000,
+		},
+		acceptsEndpoints: true,
+	}, haproxyCfg)
 }
 
 func TestLoadInvalidConfigWithNonStringArrayDimensionClients(t *testing.T) {
@@ -450,8 +481,7 @@ func TestLoadInvalidConfigWithNonStringArrayDimensionClients(t *testing.T) {
 	haproxyCfg := CreateDefaultConfig().(*Config)
 	err = cm.Unmarshal(&haproxyCfg)
 	require.Error(t, err)
-	require.ErrorContains(t, err,
-		`dimensionClients must be an array of compatible exporter names`)
+	require.ErrorContains(t, err, `expected type 'string'`)
 }
 
 func TestFilteringConfig(t *testing.T) {
@@ -465,6 +495,7 @@ func TestFilteringConfig(t *testing.T) {
 	require.NoError(t, cm.Unmarshal(&fsCfg))
 
 	require.Equal(t, &Config{
+		MonitorType: "filesystems",
 		monitorConfig: &filesystems.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type: "filesystems",
@@ -495,6 +526,7 @@ func TestInvalidFilteringConfig(t *testing.T) {
 	fsCfg := CreateDefaultConfig().(*Config)
 	require.NoError(t, cm.Unmarshal(&fsCfg))
 	require.Equal(t, &Config{
+		MonitorType: "filesystems",
 		monitorConfig: &filesystems.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type: "filesystems",
@@ -525,6 +557,7 @@ func TestLoadConfigWithNestedMonitorConfig(t *testing.T) {
 	telegrafExecCfg := CreateDefaultConfig().(*Config)
 	require.NoError(t, cm.Unmarshal(&telegrafExecCfg))
 	require.Equal(t, &Config{
+		MonitorType: "telegraf/exec",
 		monitorConfig: &exec.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "telegraf/exec",
@@ -547,6 +580,7 @@ func TestLoadConfigWithNestedMonitorConfig(t *testing.T) {
 	require.NoError(t, cm.Unmarshal(&k8sVolumesCfg))
 	tru := true
 	require.Equal(t, &Config{
+		MonitorType: "kubernetes-volumes",
 		monitorConfig: &volumes.Config{
 			MonitorConfig: saconfig.MonitorConfig{
 				Type:                "kubernetes-volumes",