Skip to content

Commit 473c9f6

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 ```
1 parent 454dff5 commit 473c9f6

File tree

4 files changed

+88
-67
lines changed

4 files changed

+88
-67
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: 24 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,13 @@ import (
2222
"strconv"
2323

2424
"github.com/signalfx/defaults"
25+
"go.opentelemetry.io/collector/confmap"
26+
"gopkg.in/yaml.v2"
27+
2528
_ "github.com/signalfx/signalfx-agent/pkg/core" // required to invoke monitor registration via init() calls
2629
saconfig "github.com/signalfx/signalfx-agent/pkg/core/config"
2730
"github.com/signalfx/signalfx-agent/pkg/core/config/validation"
2831
"github.com/signalfx/signalfx-agent/pkg/monitors"
29-
"go.opentelemetry.io/collector/confmap"
30-
"gopkg.in/yaml.v2"
3132
)
3233

3334
const defaultIntervalSeconds = 10
@@ -48,17 +49,19 @@ var (
4849
)
4950

5051
type Config struct {
51-
monitorConfig saconfig.MonitorCustomConfig
52+
MonitorType string `mapstructure:"type"` // Smart Agent monitor type, e.g. collectd/cpu
5253
// Generally an observer/receivercreator-set value via Endpoint.Target.
5354
// Will expand to MonitorCustomConfig Host and Port values if unset.
5455
Endpoint string `mapstructure:"endpoint"`
5556
DimensionClients []string `mapstructure:"dimensionClients"`
57+
58+
monitorConfig saconfig.MonitorCustomConfig
5659
acceptsEndpoints bool
5760
}
5861

5962
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")
63+
if cfg.MonitorType == "" {
64+
return fmt.Errorf(`you must specify a "type" for a smartagent receiver`)
6265
}
6366

6467
monitorConfigCore := cfg.monitorConfig.MonitorConfigCore()
@@ -77,40 +80,34 @@ func (cfg *Config) validate() error {
7780
// Unmarshal dynamically creates the desired Smart Agent monitor config
7881
// from the provided receiver config content.
7982
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")
83+
// load the non-dynamic config normally
84+
if err := componentParser.Unmarshal(cfg, confmap.WithIgnoreUnused()); err != nil {
85+
return err
8786
}
8887

89-
var endpoint any
90-
if endpoint, ok = allSettings["endpoint"]; ok {
91-
cfg.Endpoint = fmt.Sprintf("%s", endpoint)
92-
delete(allSettings, "endpoint")
88+
// No need to proceed if no monitor type is specified. The error will be caught by the validation.
89+
if cfg.MonitorType == "" {
90+
return nil
9391
}
9492

95-
var err error
96-
cfg.DimensionClients, err = getStringSliceFromAllSettings(allSettings, "dimensionClients", errDimensionClientValue)
97-
if err != nil {
98-
return err
99-
}
93+
// smartAgentConfmap is the map that will be used to create the Smart Agent Monitor config.
94+
smartAgentConfmap := componentParser.ToStringMap()
95+
delete(smartAgentConfmap, "endpoint")
96+
delete(smartAgentConfmap, "dimensionClients")
10097

10198
// monitors.ConfigTemplates is a map that all monitors use to register their custom configs in the Smart Agent.
10299
// 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)
100+
customMonitorConfig, ok := monitors.ConfigTemplates[cfg.MonitorType]
101+
if !ok {
102+
if unsupported := nonWindowsMonitors[cfg.MonitorType]; runtime.GOOS == "windows" && unsupported {
103+
return fmt.Errorf("smart agent monitor type %q is not supported on windows platforms", cfg.MonitorType)
107104
}
108-
return fmt.Errorf("no known monitor type %q", monitorType)
105+
return fmt.Errorf("no known monitor type %q", cfg.MonitorType)
109106
}
110107
monitorConfigType := reflect.TypeOf(customMonitorConfig).Elem()
111108
monitorConfig := reflect.New(monitorConfigType).Interface()
112109

113-
asBytes, err := yaml.Marshal(allSettings)
110+
asBytes, err := yaml.Marshal(smartAgentConfmap)
114111
if err != nil {
115112
return fmt.Errorf("failed constructing raw Smart Agent Monitor config block: %w", err)
116113
}
@@ -140,26 +137,6 @@ func (cfg *Config) Unmarshal(componentParser *confmap.Conf) error {
140137
return nil
141138
}
142139

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-
163140
// If using the receivercreator, observer-provided endpoints should be used to set
164141
// the Host and Port fields of monitor config structs. This can only be done by reflection without
165142
// making type assertions over all possible monitor types.

‎pkg/receiver/smartagentreceiver/config_test.go

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ import (
1919
"testing"
2020
"time"
2121

22+
"github.com/stretchr/testify/assert"
23+
"github.com/stretchr/testify/require"
24+
"go.opentelemetry.io/collector/component"
25+
"go.opentelemetry.io/collector/confmap/confmaptest"
26+
2227
"github.com/signalfx/signalfx-agent/pkg/core/common/httpclient"
2328
"github.com/signalfx/signalfx-agent/pkg/core/common/kubelet"
2429
"github.com/signalfx/signalfx-agent/pkg/core/common/kubernetes"
@@ -38,10 +43,6 @@ import (
3843
"github.com/signalfx/signalfx-agent/pkg/monitors/telegraf/monitors/exec"
3944
"github.com/signalfx/signalfx-agent/pkg/monitors/telegraf/monitors/ntpq"
4045
"github.com/signalfx/signalfx-agent/pkg/utils/timeutil"
41-
"github.com/stretchr/testify/assert"
42-
"github.com/stretchr/testify/require"
43-
"go.opentelemetry.io/collector/component"
44-
"go.opentelemetry.io/collector/confmap/confmaptest"
4546
)
4647

4748
func TestLoadConfig(t *testing.T) {
@@ -60,6 +61,7 @@ func TestLoadConfig(t *testing.T) {
6061

6162
expectedDimensionClients := []string{"nop/one", "nop/two"}
6263
require.Equal(t, &Config{
64+
MonitorType: "haproxy",
6365
DimensionClients: expectedDimensionClients,
6466
monitorConfig: &haproxy.Config{
6567
MonitorConfig: saconfig.MonitorConfig{
@@ -83,6 +85,7 @@ func TestLoadConfig(t *testing.T) {
8385
require.NoError(t, cm.Unmarshal(&redisCfg))
8486

8587
require.Equal(t, &Config{
88+
MonitorType: "collectd/redis",
8689
DimensionClients: []string{},
8790
monitorConfig: &redis.Config{
8891
MonitorConfig: saconfig.MonitorConfig{
@@ -103,6 +106,7 @@ func TestLoadConfig(t *testing.T) {
103106
require.NoError(t, cm.Unmarshal(&hadoopCfg))
104107

105108
require.Equal(t, &Config{
109+
MonitorType: "collectd/hadoop",
106110
monitorConfig: &hadoop.Config{
107111
MonitorConfig: saconfig.MonitorConfig{
108112
Type: "collectd/hadoop",
@@ -123,6 +127,7 @@ func TestLoadConfig(t *testing.T) {
123127
require.NoError(t, cm.Unmarshal(&etcdCfg))
124128

125129
require.Equal(t, &Config{
130+
MonitorType: "etcd",
126131
monitorConfig: &prometheusexporter.Config{
127132
MonitorConfig: saconfig.MonitorConfig{
128133
Type: "etcd",
@@ -147,6 +152,7 @@ func TestLoadConfig(t *testing.T) {
147152
ntpqCfg := CreateDefaultConfig().(*Config)
148153
require.NoError(t, cm.Unmarshal(&ntpqCfg))
149154
require.Equal(t, &Config{
155+
MonitorType: "telegraf/ntpq",
150156
monitorConfig: &ntpq.Config{
151157
MonitorConfig: saconfig.MonitorConfig{
152158
Type: "telegraf/ntpq",
@@ -167,6 +173,8 @@ func TestLoadInvalidConfigWithoutType(t *testing.T) {
167173
require.NoError(t, err)
168174
withoutType := CreateDefaultConfig().(*Config)
169175
err = cm.Unmarshal(&withoutType)
176+
require.NoError(t, err)
177+
err = withoutType.validate()
170178
require.Error(t, err)
171179
require.ErrorContains(t, err,
172180
`you must specify a "type" for a smartagent receiver`)
@@ -208,6 +216,7 @@ func TestLoadInvalidConfigs(t *testing.T) {
208216
negativeIntervalCfg := CreateDefaultConfig().(*Config)
209217
require.NoError(t, cm.Unmarshal(&negativeIntervalCfg))
210218
require.Equal(t, &Config{
219+
MonitorType: "collectd/redis",
211220
monitorConfig: &redis.Config{
212221
MonitorConfig: saconfig.MonitorConfig{
213222
Type: "collectd/redis",
@@ -226,6 +235,7 @@ func TestLoadInvalidConfigs(t *testing.T) {
226235
missingRequiredCfg := CreateDefaultConfig().(*Config)
227236
require.NoError(t, cm.Unmarshal(&missingRequiredCfg))
228237
require.Equal(t, &Config{
238+
MonitorType: "collectd/consul",
229239
monitorConfig: &consul.Config{
230240
MonitorConfig: saconfig.MonitorConfig{
231241
Type: "collectd/consul",
@@ -256,7 +266,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
256266
haproxyCfg := CreateDefaultConfig().(*Config)
257267
require.NoError(t, cm.Unmarshal(&haproxyCfg))
258268
require.Equal(t, &Config{
259-
Endpoint: "[fe80::20c:29ff:fe59:9446]:2345",
269+
MonitorType: "haproxy",
270+
Endpoint: "[fe80::20c:29ff:fe59:9446]:2345",
260271
monitorConfig: &haproxy.Config{
261272
MonitorConfig: saconfig.MonitorConfig{
262273
Type: "haproxy",
@@ -280,7 +291,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
280291
redisCfg := CreateDefaultConfig().(*Config)
281292
require.NoError(t, cm.Unmarshal(&redisCfg))
282293
require.Equal(t, &Config{
283-
Endpoint: "redishost",
294+
MonitorType: "collectd/redis",
295+
Endpoint: "redishost",
284296
monitorConfig: &redis.Config{
285297
MonitorConfig: saconfig.MonitorConfig{
286298
Type: "collectd/redis",
@@ -299,7 +311,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
299311
hadoopCfg := CreateDefaultConfig().(*Config)
300312
require.NoError(t, cm.Unmarshal(&hadoopCfg))
301313
require.Equal(t, &Config{
302-
Endpoint: "[::]:12345",
314+
MonitorType: "collectd/hadoop",
315+
Endpoint: "[::]:12345",
303316
monitorConfig: &hadoop.Config{
304317
MonitorConfig: saconfig.MonitorConfig{
305318
Type: "collectd/hadoop",
@@ -319,7 +332,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
319332
etcdCfg := CreateDefaultConfig().(*Config)
320333
require.NoError(t, cm.Unmarshal(&etcdCfg))
321334
require.Equal(t, &Config{
322-
Endpoint: "etcdhost:5555",
335+
MonitorType: "etcd",
336+
Endpoint: "etcdhost:5555",
323337
monitorConfig: &prometheusexporter.Config{
324338
MonitorConfig: saconfig.MonitorConfig{
325339
Type: "etcd",
@@ -344,7 +358,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
344358
require.NoError(t, cm.Unmarshal(&elasticCfg))
345359
tru := true
346360
require.Equal(t, &Config{
347-
Endpoint: "elastic:567",
361+
MonitorType: "elasticsearch",
362+
Endpoint: "elastic:567",
348363
monitorConfig: &stats.Config{
349364
MonitorConfig: saconfig.MonitorConfig{
350365
Type: "elasticsearch",
@@ -373,7 +388,8 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
373388
kubeletCfg := CreateDefaultConfig().(*Config)
374389
require.NoError(t, cm.Unmarshal(&kubeletCfg))
375390
require.Equal(t, &Config{
376-
Endpoint: "disregarded:678",
391+
MonitorType: "kubelet-stats",
392+
Endpoint: "disregarded:678",
377393
monitorConfig: &cadvisor.KubeletStatsConfig{
378394
MonitorConfig: saconfig.MonitorConfig{
379395
Type: "kubelet-stats",
@@ -415,7 +431,8 @@ func TestLoadConfigWithUnsupportedEndpoint(t *testing.T) {
415431
require.NoError(t, cm.Unmarshal(&nagiosCfg))
416432

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

445477
func TestLoadInvalidConfigWithNonStringArrayDimensionClients(t *testing.T) {
@@ -450,8 +482,7 @@ func TestLoadInvalidConfigWithNonStringArrayDimensionClients(t *testing.T) {
450482
haproxyCfg := CreateDefaultConfig().(*Config)
451483
err = cm.Unmarshal(&haproxyCfg)
452484
require.Error(t, err)
453-
require.ErrorContains(t, err,
454-
`dimensionClients must be an array of compatible exporter names`)
485+
require.ErrorContains(t, err, `expected type 'string'`)
455486
}
456487

457488
func TestFilteringConfig(t *testing.T) {
@@ -465,6 +496,7 @@ func TestFilteringConfig(t *testing.T) {
465496
require.NoError(t, cm.Unmarshal(&fsCfg))
466497

467498
require.Equal(t, &Config{
499+
MonitorType: "filesystems",
468500
monitorConfig: &filesystems.Config{
469501
MonitorConfig: saconfig.MonitorConfig{
470502
Type: "filesystems",
@@ -495,6 +527,7 @@ func TestInvalidFilteringConfig(t *testing.T) {
495527
fsCfg := CreateDefaultConfig().(*Config)
496528
require.NoError(t, cm.Unmarshal(&fsCfg))
497529
require.Equal(t, &Config{
530+
MonitorType: "filesystems",
498531
monitorConfig: &filesystems.Config{
499532
MonitorConfig: saconfig.MonitorConfig{
500533
Type: "filesystems",
@@ -525,6 +558,7 @@ func TestLoadConfigWithNestedMonitorConfig(t *testing.T) {
525558
telegrafExecCfg := CreateDefaultConfig().(*Config)
526559
require.NoError(t, cm.Unmarshal(&telegrafExecCfg))
527560
require.Equal(t, &Config{
561+
MonitorType: "telegraf/exec",
528562
monitorConfig: &exec.Config{
529563
MonitorConfig: saconfig.MonitorConfig{
530564
Type: "telegraf/exec",
@@ -547,6 +581,7 @@ func TestLoadConfigWithNestedMonitorConfig(t *testing.T) {
547581
require.NoError(t, cm.Unmarshal(&k8sVolumesCfg))
548582
tru := true
549583
require.Equal(t, &Config{
584+
MonitorType: "kubernetes-volumes",
550585
monitorConfig: &volumes.Config{
551586
MonitorConfig: saconfig.MonitorConfig{
552587
Type: "kubernetes-volumes",

0 commit comments

Comments
 (0)