Skip to content

Commit 5109a48

Browse files
dmitryaxdmitryax
committed
Move smart agent config validation from unmarshal
This fixes receiver_creator failing to start smartagent receivers since 0.124.0 with the following error: ``` 2025-05-01T09:35:43.989Z info [email protected]/observerhandler.go:201 starting receiver {"name": "smartagent/coredns", "endpoint": "10.42.0.8", "endpoint_id": "k8s_observer/c7a1646a-969d-4e5d-9bbd-a95574b903b3", "config": {"extraDimensions":{"metric_source":"k8s-coredns"},"port":9153,"type":"coredns"}} 2025-05-01T09:35:43.991Z error [email protected]/observerhandler.go:217 failed to start receiver {"receiver": "smartagent/coredns", "error": "failed creating endpoint-derived receiver: Validation error in field 'Config.host': host is a required field (got '')"} github.com/open-telemetry/opentelemetry-collector-contrib/receiver/receivercreator.(*observerHandler).startReceiver github.com/open-telemetry/opentelemetry-collector-contrib/receiver/[email protected]/observerhandler.go:217 github.com/open-telemetry/opentelemetry-collector-contrib/receiver/receivercreator.(*observerHandler).OnAdd github.com/open-telemetry/opentelemetry-collector-contrib/receiver/[email protected]/observerhandler.go:105 github.com/open-telemetry/opentelemetry-collector-contrib/extension/observer/endpointswatcher.(*EndpointsWatcher).updateAndNotifyOfEndpoints github.com/open-telemetry/opentelemetry-collector-contrib/extension/[email protected]/endpointswatcher/endpointswatcher.go:114 ``` 454dff
1 parent 454dff5 commit 5109a48

File tree

6 files changed

+88
-64
lines changed

6 files changed

+88
-64
lines changed

‎CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## Unreleased
44

5+
### 🧰 Bug fixes 🧰
6+
7+
- (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))
8+
59
## v0.124.0
610

711
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

Lines changed: 20 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,19 @@ var (
4848
)
4949

5050
type Config struct {
51-
monitorConfig saconfig.MonitorCustomConfig
51+
MonitorType string `mapstructure:"type"` // Smart Agent monitor type, e.g. collectd/cpu
5252
// Generally an observer/receivercreator-set value via Endpoint.Target.
5353
// Will expand to MonitorCustomConfig Host and Port values if unset.
5454
Endpoint string `mapstructure:"endpoint"`
5555
DimensionClients []string `mapstructure:"dimensionClients"`
56+
57+
monitorConfig saconfig.MonitorCustomConfig
5658
acceptsEndpoints bool
5759
}
5860

5961
func (cfg *Config) validate() error {
60-
if cfg == nil || cfg.monitorConfig == nil {
61-
return fmt.Errorf("you must supply a valid Smart Agent Monitor config")
62+
if cfg.MonitorType == "" {
63+
return fmt.Errorf(`you must specify a "type" for a smartagent receiver`)
6264
}
6365

6466
monitorConfigCore := cfg.monitorConfig.MonitorConfigCore()
@@ -77,40 +79,33 @@ func (cfg *Config) validate() error {
7779
// Unmarshal dynamically creates the desired Smart Agent monitor config
7880
// from the provided receiver config content.
7981
func (cfg *Config) Unmarshal(componentParser *confmap.Conf) error {
80-
// AllSettings() is the user provided config and intoCfg is the default Config instance we populate to
81-
// form the final desired version. To do so, we manually obtain all Config items, leaving only Smart Agent
82-
// monitor config settings to be unmarshalled to their respective custom monitor config types.
83-
allSettings := componentParser.ToStringMap()
84-
monitorType, ok := allSettings["type"].(string)
85-
if !ok || monitorType == "" {
86-
return fmt.Errorf("you must specify a \"type\" for a smartagent receiver")
82+
// Load the non-dynamic config normally ignoring unused fields.
83+
if err := componentParser.Unmarshal(cfg, confmap.WithIgnoreUnused()); err != nil {
84+
return err
8785
}
8886

89-
var endpoint any
90-
if endpoint, ok = allSettings["endpoint"]; ok {
91-
cfg.Endpoint = fmt.Sprintf("%s", endpoint)
92-
delete(allSettings, "endpoint")
87+
// No need to proceed if monitor type isn't specified, this will fail the validation.
88+
if cfg.MonitorType == "" {
89+
return nil
9390
}
9491

95-
var err error
96-
cfg.DimensionClients, err = getStringSliceFromAllSettings(allSettings, "dimensionClients", errDimensionClientValue)
97-
if err != nil {
98-
return err
99-
}
92+
monitorConfmap := componentParser.ToStringMap()
93+
delete(monitorConfmap, "endpoint")
94+
delete(monitorConfmap, "dimensionClients")
10095

10196
// monitors.ConfigTemplates is a map that all monitors use to register their custom configs in the Smart Agent.
10297
// The values are always pointers to an actual custom config.
103-
var customMonitorConfig saconfig.MonitorCustomConfig
104-
if customMonitorConfig, ok = monitors.ConfigTemplates[monitorType]; !ok {
105-
if unsupported := nonWindowsMonitors[monitorType]; runtime.GOOS == "windows" && unsupported {
106-
return fmt.Errorf("smart agent monitor type %q is not supported on windows platforms", monitorType)
98+
customMonitorConfig, ok := monitors.ConfigTemplates[cfg.MonitorType]
99+
if !ok {
100+
if unsupported := nonWindowsMonitors[cfg.MonitorType]; runtime.GOOS == "windows" && unsupported {
101+
return fmt.Errorf("smart agent monitor type %q is not supported on windows platforms", cfg.MonitorType)
107102
}
108-
return fmt.Errorf("no known monitor type %q", monitorType)
103+
return fmt.Errorf("no known monitor type %q", cfg.MonitorType)
109104
}
110105
monitorConfigType := reflect.TypeOf(customMonitorConfig).Elem()
111106
monitorConfig := reflect.New(monitorConfigType).Interface()
112107

113-
asBytes, err := yaml.Marshal(allSettings)
108+
asBytes, err := yaml.Marshal(monitorConfmap)
114109
if err != nil {
115110
return fmt.Errorf("failed constructing raw Smart Agent Monitor config block: %w", err)
116111
}
@@ -140,26 +135,6 @@ func (cfg *Config) Unmarshal(componentParser *confmap.Conf) error {
140135
return nil
141136
}
142137

143-
func getStringSliceFromAllSettings(allSettings map[string]any, key string, errToReturn error) ([]string, error) {
144-
var items []string
145-
if value, ok := allSettings[key]; ok {
146-
items = []string{}
147-
if valueAsSlice, isSlice := value.([]any); isSlice {
148-
for _, c := range valueAsSlice {
149-
if client, isString := c.(string); isString {
150-
items = append(items, client)
151-
} else {
152-
return nil, errToReturn
153-
}
154-
}
155-
} else {
156-
return nil, errToReturn
157-
}
158-
delete(allSettings, key)
159-
}
160-
return items, nil
161-
}
162-
163138
// If using the receivercreator, observer-provided endpoints should be used to set
164139
// the Host and Port fields of monitor config structs. This can only be done by reflection without
165140
// making type assertions over all possible monitor types.

‎pkg/receiver/smartagentreceiver/config_linux_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ func TestLoadConfigWithLinuxOnlyMonitors(t *testing.T) {
4444
err = cm.Unmarshal(&apacheCfg)
4545
require.NoError(t, err)
4646
require.Equal(t, &Config{
47+
MonitorType: "collectd/apache",
4748
monitorConfig: &apache.Config{
4849
MonitorConfig: saconfig.MonitorConfig{
4950
Type: "collectd/apache",
@@ -64,6 +65,7 @@ func TestLoadConfigWithLinuxOnlyMonitors(t *testing.T) {
6465
err = cm.Unmarshal(&kafkaCfg)
6566
require.NoError(t, err)
6667
require.Equal(t, &Config{
68+
MonitorType: "collectd/kafka",
6769
monitorConfig: &kafka.Config{
6870
Config: genericjmx.Config{
6971
MonitorConfig: saconfig.MonitorConfig{
@@ -87,6 +89,7 @@ func TestLoadConfigWithLinuxOnlyMonitors(t *testing.T) {
8789
err = cm.Unmarshal(&memcachedCfg)
8890
require.NoError(t, err)
8991
require.Equal(t, &Config{
92+
MonitorType: "collectd/memcached",
9093
monitorConfig: &memcached.Config{
9194
MonitorConfig: saconfig.MonitorConfig{
9295
Type: "collectd/memcached",
@@ -106,6 +109,7 @@ func TestLoadConfigWithLinuxOnlyMonitors(t *testing.T) {
106109
err = cm.Unmarshal(&phpCfg)
107110
require.NoError(t, err)
108111
require.Equal(t, &Config{
112+
MonitorType: "collectd/php-fpm",
109113
monitorConfig: &php.Config{
110114
MonitorConfig: saconfig.MonitorConfig{
111115
Type: "collectd/php-fpm",

‎pkg/receiver/smartagentreceiver/config_test.go

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ func TestLoadConfig(t *testing.T) {
6060

6161
expectedDimensionClients := []string{"nop/one", "nop/two"}
6262
require.Equal(t, &Config{
63+
MonitorType: "haproxy",
6364
DimensionClients: expectedDimensionClients,
6465
monitorConfig: &haproxy.Config{
6566
MonitorConfig: saconfig.MonitorConfig{
@@ -83,6 +84,7 @@ func TestLoadConfig(t *testing.T) {
8384
require.NoError(t, cm.Unmarshal(&redisCfg))
8485

8586
require.Equal(t, &Config{
87+
MonitorType: "collectd/redis",
8688
DimensionClients: []string{},
8789
monitorConfig: &redis.Config{
8890
MonitorConfig: saconfig.MonitorConfig{
@@ -103,6 +105,7 @@ func TestLoadConfig(t *testing.T) {
103105
require.NoError(t, cm.Unmarshal(&hadoopCfg))
104106

105107
require.Equal(t, &Config{
108+
MonitorType: "collectd/hadoop",
106109
monitorConfig: &hadoop.Config{
107110
MonitorConfig: saconfig.MonitorConfig{
108111
Type: "collectd/hadoop",
@@ -123,6 +126,7 @@ func TestLoadConfig(t *testing.T) {
123126
require.NoError(t, cm.Unmarshal(&etcdCfg))
124127

125128
require.Equal(t, &Config{
129+
MonitorType: "etcd",
126130
monitorConfig: &prometheusexporter.Config{
127131
MonitorConfig: saconfig.MonitorConfig{
128132
Type: "etcd",
@@ -147,6 +151,7 @@ func TestLoadConfig(t *testing.T) {
147151
ntpqCfg := CreateDefaultConfig().(*Config)
148152
require.NoError(t, cm.Unmarshal(&ntpqCfg))
149153
require.Equal(t, &Config{
154+
MonitorType: "telegraf/ntpq",
150155
monitorConfig: &ntpq.Config{
151156
MonitorConfig: saconfig.MonitorConfig{
152157
Type: "telegraf/ntpq",
@@ -167,6 +172,8 @@ func TestLoadInvalidConfigWithoutType(t *testing.T) {
167172
require.NoError(t, err)
168173
withoutType := CreateDefaultConfig().(*Config)
169174
err = cm.Unmarshal(&withoutType)
175+
require.NoError(t, err)
176+
err = withoutType.validate()
170177
require.Error(t, err)
171178
require.ErrorContains(t, err,
172179
`you must specify a "type" for a smartagent receiver`)
@@ -208,6 +215,7 @@ func TestLoadInvalidConfigs(t *testing.T) {
208215
negativeIntervalCfg := CreateDefaultConfig().(*Config)
209216
require.NoError(t, cm.Unmarshal(&negativeIntervalCfg))
210217
require.Equal(t, &Config{
218+
MonitorType: "collectd/redis",
211219
monitorConfig: &redis.Config{
212220
MonitorConfig: saconfig.MonitorConfig{
213221
Type: "collectd/redis",
@@ -226,6 +234,7 @@ func TestLoadInvalidConfigs(t *testing.T) {
226234
missingRequiredCfg := CreateDefaultConfig().(*Config)
227235
require.NoError(t, cm.Unmarshal(&missingRequiredCfg))
228236
require.Equal(t, &Config{
237+
MonitorType: "collectd/consul",
229238
monitorConfig: &consul.Config{
230239
MonitorConfig: saconfig.MonitorConfig{
231240
Type: "collectd/consul",
@@ -256,7 +265,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
256265
haproxyCfg := CreateDefaultConfig().(*Config)
257266
require.NoError(t, cm.Unmarshal(&haproxyCfg))
258267
require.Equal(t, &Config{
259-
Endpoint: "[fe80::20c:29ff:fe59:9446]:2345",
268+
MonitorType: "haproxy",
269+
Endpoint: "[fe80::20c:29ff:fe59:9446]:2345",
260270
monitorConfig: &haproxy.Config{
261271
MonitorConfig: saconfig.MonitorConfig{
262272
Type: "haproxy",
@@ -280,7 +290,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
280290
redisCfg := CreateDefaultConfig().(*Config)
281291
require.NoError(t, cm.Unmarshal(&redisCfg))
282292
require.Equal(t, &Config{
283-
Endpoint: "redishost",
293+
MonitorType: "collectd/redis",
294+
Endpoint: "redishost",
284295
monitorConfig: &redis.Config{
285296
MonitorConfig: saconfig.MonitorConfig{
286297
Type: "collectd/redis",
@@ -299,7 +310,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
299310
hadoopCfg := CreateDefaultConfig().(*Config)
300311
require.NoError(t, cm.Unmarshal(&hadoopCfg))
301312
require.Equal(t, &Config{
302-
Endpoint: "[::]:12345",
313+
MonitorType: "collectd/hadoop",
314+
Endpoint: "[::]:12345",
303315
monitorConfig: &hadoop.Config{
304316
MonitorConfig: saconfig.MonitorConfig{
305317
Type: "collectd/hadoop",
@@ -319,7 +331,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
319331
etcdCfg := CreateDefaultConfig().(*Config)
320332
require.NoError(t, cm.Unmarshal(&etcdCfg))
321333
require.Equal(t, &Config{
322-
Endpoint: "etcdhost:5555",
334+
MonitorType: "etcd",
335+
Endpoint: "etcdhost:5555",
323336
monitorConfig: &prometheusexporter.Config{
324337
MonitorConfig: saconfig.MonitorConfig{
325338
Type: "etcd",
@@ -344,7 +357,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
344357
require.NoError(t, cm.Unmarshal(&elasticCfg))
345358
tru := true
346359
require.Equal(t, &Config{
347-
Endpoint: "elastic:567",
360+
MonitorType: "elasticsearch",
361+
Endpoint: "elastic:567",
348362
monitorConfig: &stats.Config{
349363
MonitorConfig: saconfig.MonitorConfig{
350364
Type: "elasticsearch",
@@ -373,7 +387,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
373387
kubeletCfg := CreateDefaultConfig().(*Config)
374388
require.NoError(t, cm.Unmarshal(&kubeletCfg))
375389
require.Equal(t, &Config{
376-
Endpoint: "disregarded:678",
390+
MonitorType: "kubelet-stats",
391+
Endpoint: "disregarded:678",
377392
monitorConfig: &cadvisor.KubeletStatsConfig{
378393
MonitorConfig: saconfig.MonitorConfig{
379394
Type: "kubelet-stats",
@@ -415,7 +430,8 @@ func TestLoadConfigWithUnsupportedEndpoint(t *testing.T) {
415430
require.NoError(t, cm.Unmarshal(&nagiosCfg))
416431

417432
require.Equal(t, &Config{
418-
Endpoint: "localhost:12345",
433+
MonitorType: "nagios",
434+
Endpoint: "localhost:12345",
419435
monitorConfig: &nagios.Config{
420436
MonitorConfig: saconfig.MonitorConfig{
421437
Type: "nagios",
@@ -437,9 +453,24 @@ func TestLoadInvalidConfigWithNonArrayDimensionClients(t *testing.T) {
437453
require.NoError(t, err)
438454
haproxyCfg := CreateDefaultConfig().(*Config)
439455
err = cm.Unmarshal(&haproxyCfg)
440-
require.Error(t, err)
441-
require.ErrorContains(t, err,
442-
`dimensionClients must be an array of compatible exporter names`)
456+
require.NoError(t, err)
457+
require.Equal(t, &Config{
458+
MonitorType: "haproxy",
459+
DimensionClients: []string{"notanarray"},
460+
monitorConfig: &haproxy.Config{
461+
MonitorConfig: saconfig.MonitorConfig{
462+
Type: "haproxy",
463+
DatapointsToExclude: []saconfig.MetricFilter{},
464+
IntervalSeconds: 123,
465+
},
466+
Username: "SomeUser",
467+
Password: "secret",
468+
Path: "stats?stats;csv",
469+
SSLVerify: true,
470+
Timeout: 5000000000,
471+
},
472+
acceptsEndpoints: true,
473+
}, haproxyCfg)
443474
}
444475

445476
func TestLoadInvalidConfigWithNonStringArrayDimensionClients(t *testing.T) {
@@ -450,8 +481,7 @@ func TestLoadInvalidConfigWithNonStringArrayDimensionClients(t *testing.T) {
450481
haproxyCfg := CreateDefaultConfig().(*Config)
451482
err = cm.Unmarshal(&haproxyCfg)
452483
require.Error(t, err)
453-
require.ErrorContains(t, err,
454-
`dimensionClients must be an array of compatible exporter names`)
484+
require.ErrorContains(t, err, `expected type 'string'`)
455485
}
456486

457487
func TestFilteringConfig(t *testing.T) {
@@ -465,6 +495,7 @@ func TestFilteringConfig(t *testing.T) {
465495
require.NoError(t, cm.Unmarshal(&fsCfg))
466496

467497
require.Equal(t, &Config{
498+
MonitorType: "filesystems",
468499
monitorConfig: &filesystems.Config{
469500
MonitorConfig: saconfig.MonitorConfig{
470501
Type: "filesystems",
@@ -495,6 +526,7 @@ func TestInvalidFilteringConfig(t *testing.T) {
495526
fsCfg := CreateDefaultConfig().(*Config)
496527
require.NoError(t, cm.Unmarshal(&fsCfg))
497528
require.Equal(t, &Config{
529+
MonitorType: "filesystems",
498530
monitorConfig: &filesystems.Config{
499531
MonitorConfig: saconfig.MonitorConfig{
500532
Type: "filesystems",
@@ -525,6 +557,7 @@ func TestLoadConfigWithNestedMonitorConfig(t *testing.T) {
525557
telegrafExecCfg := CreateDefaultConfig().(*Config)
526558
require.NoError(t, cm.Unmarshal(&telegrafExecCfg))
527559
require.Equal(t, &Config{
560+
MonitorType: "telegraf/exec",
528561
monitorConfig: &exec.Config{
529562
MonitorConfig: saconfig.MonitorConfig{
530563
Type: "telegraf/exec",
@@ -547,6 +580,7 @@ func TestLoadConfigWithNestedMonitorConfig(t *testing.T) {
547580
require.NoError(t, cm.Unmarshal(&k8sVolumesCfg))
548581
tru := true
549582
require.Equal(t, &Config{
583+
MonitorType: "kubernetes-volumes",
550584
monitorConfig: &volumes.Config{
551585
MonitorConfig: saconfig.MonitorConfig{
552586
Type: "kubernetes-volumes",

0 commit comments

Comments
 (0)