diff --git a/CHANGELOG.md b/CHANGELOG.md index e5023c702e..582717f037 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/pkg/receiver/smartagentreceiver/config.go b/pkg/receiver/smartagentreceiver/config.go index 1500ed5882..6b3e144e3c 100644 --- a/pkg/receiver/smartagentreceiver/config.go +++ b/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. diff --git a/pkg/receiver/smartagentreceiver/config_linux_test.go b/pkg/receiver/smartagentreceiver/config_linux_test.go index 1a9e3dee8f..d945782fad 100644 --- a/pkg/receiver/smartagentreceiver/config_linux_test.go +++ b/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", diff --git a/pkg/receiver/smartagentreceiver/config_test.go b/pkg/receiver/smartagentreceiver/config_test.go index 63e583f09e..a353188b61 100644 --- a/pkg/receiver/smartagentreceiver/config_test.go +++ b/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", diff --git a/pkg/receiver/smartagentreceiver/factory_test.go b/pkg/receiver/smartagentreceiver/factory_test.go index 0f2eed425c..d9ff360ba9 100644 --- a/pkg/receiver/smartagentreceiver/factory_test.go +++ b/pkg/receiver/smartagentreceiver/factory_test.go @@ -37,6 +37,7 @@ func TestCreateDefaultConfig(t *testing.T) { func TestCreateMetrics(t *testing.T) { factory := NewFactory() cfg := factory.CreateDefaultConfig() + cfg.(*Config).MonitorType = "haproxy" cfg.(*Config).monitorConfig = &haproxy.Config{} params := otelcolreceiver.Settings{ID: component.MustNewID(typeStr)} @@ -55,7 +56,7 @@ func TestCreateMetricsWithInvalidConfig(t *testing.T) { params := otelcolreceiver.Settings{ID: component.MustNewID(typeStr)} receiver, err := factory.CreateMetrics(context.Background(), params, cfg, consumertest.NewNop()) require.Error(t, err) - assert.EqualError(t, err, "you must supply a valid Smart Agent Monitor config") + assert.EqualError(t, err, `you must specify a "type" for a smartagent receiver`) assert.Nil(t, receiver) assert.NotContains(t, receiverStore, cfg) @@ -64,6 +65,7 @@ func TestCreateMetricsWithInvalidConfig(t *testing.T) { func TestCreateLogs(t *testing.T) { factory := NewFactory() cfg := factory.CreateDefaultConfig() + cfg.(*Config).MonitorType = "haproxy" cfg.(*Config).monitorConfig = &haproxy.Config{} params := otelcolreceiver.Settings{ID: component.MustNewID(typeStr)} @@ -82,7 +84,7 @@ func TestCreateLogsWithInvalidConfig(t *testing.T) { params := otelcolreceiver.Settings{ID: component.MustNewID(typeStr)} receiver, err := factory.CreateLogs(context.Background(), params, cfg, consumertest.NewNop()) require.Error(t, err) - assert.EqualError(t, err, "you must supply a valid Smart Agent Monitor config") + assert.EqualError(t, err, `you must specify a "type" for a smartagent receiver`) assert.Nil(t, receiver) assert.NotContains(t, receiverStore, cfg) @@ -91,6 +93,7 @@ func TestCreateLogsWithInvalidConfig(t *testing.T) { func TestCreateTraces(t *testing.T) { factory := NewFactory() cfg := factory.CreateDefaultConfig() + cfg.(*Config).MonitorType = "haproxy" cfg.(*Config).monitorConfig = &haproxy.Config{} params := otelcolreceiver.Settings{ID: component.MustNewID(typeStr)} @@ -109,7 +112,7 @@ func TestCreateTracesWithInvalidConfig(t *testing.T) { params := otelcolreceiver.Settings{ID: component.MustNewID(typeStr)} receiver, err := factory.CreateTraces(context.Background(), params, cfg, consumertest.NewNop()) require.Error(t, err) - assert.EqualError(t, err, "you must supply a valid Smart Agent Monitor config") + assert.EqualError(t, err, `you must specify a "type" for a smartagent receiver`) assert.Nil(t, receiver) assert.NotContains(t, receiverStore, cfg) @@ -118,6 +121,7 @@ func TestCreateTracesWithInvalidConfig(t *testing.T) { func TestCreateMetricsThenLogsAndThenTracesReceiver(t *testing.T) { factory := NewFactory() cfg := factory.CreateDefaultConfig() + cfg.(*Config).MonitorType = "haproxy" cfg.(*Config).monitorConfig = &haproxy.Config{} params := otelcolreceiver.Settings{ID: component.MustNewID(typeStr)} @@ -148,6 +152,7 @@ func TestCreateMetricsThenLogsAndThenTracesReceiver(t *testing.T) { func TestCreateTracesThenLogsAndThenMetricsReceiver(t *testing.T) { factory := NewFactory() cfg := factory.CreateDefaultConfig() + cfg.(*Config).MonitorType = "haproxy" cfg.(*Config).monitorConfig = &haproxy.Config{} params := otelcolreceiver.Settings{ID: component.MustNewID(typeStr)} diff --git a/pkg/receiver/smartagentreceiver/receiver_test.go b/pkg/receiver/smartagentreceiver/receiver_test.go index 837639e6fb..4b167934ea 100644 --- a/pkg/receiver/smartagentreceiver/receiver_test.go +++ b/pkg/receiver/smartagentreceiver/receiver_test.go @@ -26,10 +26,6 @@ import ( "testing" "time" - saconfig "github.com/signalfx/signalfx-agent/pkg/core/config" - "github.com/signalfx/signalfx-agent/pkg/monitors" - "github.com/signalfx/signalfx-agent/pkg/monitors/cpu" - "github.com/signalfx/signalfx-agent/pkg/utils/hostfs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" @@ -48,6 +44,11 @@ import ( "go.uber.org/zap/zapcore" "go.uber.org/zap/zaptest/observer" + saconfig "github.com/signalfx/signalfx-agent/pkg/core/config" + "github.com/signalfx/signalfx-agent/pkg/monitors" + "github.com/signalfx/signalfx-agent/pkg/monitors/cpu" + "github.com/signalfx/signalfx-agent/pkg/utils/hostfs" + "github.com/signalfx/splunk-otel-collector/pkg/extension/smartagentextension" ) @@ -95,6 +96,7 @@ var ( func newConfig(monitorType string, intervalSeconds int) Config { return Config{ + MonitorType: monitorType, monitorConfig: &cpu.Config{ MonitorConfig: saconfig.MonitorConfig{ Type: monitorType,