Skip to content

Commit 4068b24

Browse files
dmitryaxdmitryax
authored
[chore] [receiver/smartagent] Don't call validate explicitly (#6192)
Implement xconfmap.Validator interface instead so it's called automatically after unmarshaling
1 parent b384cb7 commit 4068b24

File tree

9 files changed

+36
-87
lines changed

9 files changed

+36
-87
lines changed

‎pkg/receiver/smartagentreceiver/config.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,15 @@ import (
2727
"github.com/signalfx/signalfx-agent/pkg/core/config/validation"
2828
"github.com/signalfx/signalfx-agent/pkg/monitors"
2929
"go.opentelemetry.io/collector/confmap"
30+
"go.opentelemetry.io/collector/confmap/xconfmap"
3031
"gopkg.in/yaml.v2"
3132
)
3233

3334
const defaultIntervalSeconds = 10
3435

3536
var (
3637
_ confmap.Unmarshaler = (*Config)(nil)
38+
_ xconfmap.Validator = (*Config)(nil)
3739

3840
nonWindowsMonitors = map[string]bool{
3941
"collectd/activemq": true, "collectd/apache": true, "collectd/cassandra": true, "collectd/chrony": true,
@@ -56,7 +58,7 @@ type Config struct {
5658
acceptsEndpoints bool
5759
}
5860

59-
func (cfg *Config) validate() error {
61+
func (cfg *Config) Validate() error {
6062
if cfg.MonitorType == "" {
6163
return fmt.Errorf(`you must specify a "type" for a smartagent receiver`)
6264
}

‎pkg/receiver/smartagentreceiver/config_linux_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func TestLoadConfigWithLinuxOnlyMonitors(t *testing.T) {
5757
},
5858
acceptsEndpoints: true,
5959
}, apacheCfg)
60-
require.NoError(t, apacheCfg.validate())
60+
require.NoError(t, apacheCfg.Validate())
6161

6262
cm, err = configs.Sub(component.MustNewIDWithName(typeStr, "kafka").String())
6363
require.NoError(t, err)
@@ -81,7 +81,7 @@ func TestLoadConfigWithLinuxOnlyMonitors(t *testing.T) {
8181
},
8282
acceptsEndpoints: true,
8383
}, kafkaCfg)
84-
require.NoError(t, kafkaCfg.validate())
84+
require.NoError(t, kafkaCfg.Validate())
8585

8686
cm, err = configs.Sub(component.MustNewIDWithName(typeStr, "memcached").String())
8787
require.NoError(t, err)
@@ -101,7 +101,7 @@ func TestLoadConfigWithLinuxOnlyMonitors(t *testing.T) {
101101
},
102102
acceptsEndpoints: true,
103103
}, memcachedCfg)
104-
require.NoError(t, memcachedCfg.validate())
104+
require.NoError(t, memcachedCfg.Validate())
105105

106106
cm, err = configs.Sub(component.MustNewIDWithName(typeStr, "php").String())
107107
require.NoError(t, err)
@@ -120,5 +120,5 @@ func TestLoadConfigWithLinuxOnlyMonitors(t *testing.T) {
120120
},
121121
acceptsEndpoints: true,
122122
}, phpCfg)
123-
require.NoError(t, phpCfg.validate())
123+
require.NoError(t, phpCfg.Validate())
124124
}

‎pkg/receiver/smartagentreceiver/config_test.go

+25-19
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func TestLoadConfig(t *testing.T) {
7676
},
7777
acceptsEndpoints: true,
7878
}, haproxyCfg)
79-
require.NoError(t, haproxyCfg.validate())
79+
require.NoError(t, haproxyCfg.Validate())
8080

8181
cm, err = cfg.Sub(component.MustNewIDWithName(typeStr, "redis").String())
8282
require.NoError(t, err)
@@ -97,7 +97,7 @@ func TestLoadConfig(t *testing.T) {
9797
},
9898
acceptsEndpoints: true,
9999
}, redisCfg)
100-
require.NoError(t, redisCfg.validate())
100+
require.NoError(t, redisCfg.Validate())
101101

102102
cm, err = cfg.Sub(component.MustNewIDWithName(typeStr, "hadoop").String())
103103
require.NoError(t, err)
@@ -118,7 +118,7 @@ func TestLoadConfig(t *testing.T) {
118118
},
119119
acceptsEndpoints: true,
120120
}, hadoopCfg)
121-
require.NoError(t, hadoopCfg.validate())
121+
require.NoError(t, hadoopCfg.Validate())
122122

123123
cm, err = cfg.Sub(component.MustNewIDWithName(typeStr, "etcd").String())
124124
require.NoError(t, err)
@@ -143,7 +143,7 @@ func TestLoadConfig(t *testing.T) {
143143
},
144144
acceptsEndpoints: true,
145145
}, etcdCfg)
146-
require.NoError(t, etcdCfg.validate())
146+
require.NoError(t, etcdCfg.Validate())
147147

148148
tr := true
149149
cm, err = cfg.Sub(component.MustNewIDWithName(typeStr, "ntpq").String())
@@ -162,7 +162,7 @@ func TestLoadConfig(t *testing.T) {
162162
},
163163
acceptsEndpoints: true,
164164
}, ntpqCfg)
165-
require.NoError(t, ntpqCfg.validate())
165+
require.NoError(t, ntpqCfg.Validate())
166166
}
167167

168168
func TestLoadInvalidConfigWithoutType(t *testing.T) {
@@ -173,7 +173,7 @@ func TestLoadInvalidConfigWithoutType(t *testing.T) {
173173
withoutType := CreateDefaultConfig().(*Config)
174174
err = cm.Unmarshal(&withoutType)
175175
require.NoError(t, err)
176-
err = withoutType.validate()
176+
err = withoutType.Validate()
177177
require.Error(t, err)
178178
require.ErrorContains(t, err,
179179
`you must specify a "type" for a smartagent receiver`)
@@ -225,7 +225,7 @@ func TestLoadInvalidConfigs(t *testing.T) {
225225
},
226226
acceptsEndpoints: true,
227227
}, negativeIntervalCfg)
228-
err = negativeIntervalCfg.validate()
228+
err = negativeIntervalCfg.Validate()
229229
require.Error(t, err)
230230
require.EqualError(t, err, "intervalSeconds must be greater than 0s (-234 provided)")
231231

@@ -247,7 +247,7 @@ func TestLoadInvalidConfigs(t *testing.T) {
247247
},
248248
acceptsEndpoints: true,
249249
}, missingRequiredCfg)
250-
err = missingRequiredCfg.validate()
250+
err = missingRequiredCfg.Validate()
251251
require.Error(t, err)
252252
require.EqualError(t, err, "Validation error in field 'Config.host': host is a required field (got '')")
253253
}
@@ -283,7 +283,7 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
283283
},
284284
acceptsEndpoints: true,
285285
}, haproxyCfg)
286-
require.NoError(t, haproxyCfg.validate())
286+
require.NoError(t, haproxyCfg.Validate())
287287

288288
cm, err = cfg.Sub(component.MustNewIDWithName(typeStr, "redis").String())
289289
require.NoError(t, err)
@@ -303,7 +303,7 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
303303
},
304304
acceptsEndpoints: true,
305305
}, redisCfg)
306-
require.NoError(t, redisCfg.validate())
306+
require.NoError(t, redisCfg.Validate())
307307

308308
cm, err = cfg.Sub(component.MustNewIDWithName(typeStr, "hadoop").String())
309309
require.NoError(t, err)
@@ -324,7 +324,7 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
324324
},
325325
acceptsEndpoints: true,
326326
}, hadoopCfg)
327-
require.NoError(t, hadoopCfg.validate())
327+
require.NoError(t, hadoopCfg.Validate())
328328

329329
cm, err = cfg.Sub(component.MustNewIDWithName(typeStr, "etcd").String())
330330
require.NoError(t, err)
@@ -349,7 +349,7 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
349349
},
350350
acceptsEndpoints: true,
351351
}, etcdCfg)
352-
require.NoError(t, etcdCfg.validate())
352+
require.NoError(t, etcdCfg.Validate())
353353

354354
cm, err = cfg.Sub(component.MustNewIDWithName(typeStr, "elasticsearch").String())
355355
require.NoError(t, err)
@@ -380,7 +380,7 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
380380
},
381381
acceptsEndpoints: true,
382382
}, elasticCfg)
383-
require.NoError(t, elasticCfg.validate())
383+
require.NoError(t, elasticCfg.Validate())
384384

385385
cm, err = cfg.Sub(component.MustNewIDWithName(typeStr, "kubelet-stats").String())
386386
require.NoError(t, err)
@@ -402,7 +402,7 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
402402
},
403403
acceptsEndpoints: true,
404404
}, kubeletCfg)
405-
require.NoError(t, kubeletCfg.validate())
405+
require.NoError(t, kubeletCfg.Validate())
406406
}
407407

408408
func TestLoadInvalidConfigWithInvalidEndpoint(t *testing.T) {
@@ -443,7 +443,7 @@ func TestLoadConfigWithUnsupportedEndpoint(t *testing.T) {
443443
},
444444
acceptsEndpoints: false,
445445
}, nagiosCfg)
446-
require.NoError(t, nagiosCfg.validate())
446+
require.NoError(t, nagiosCfg.Validate())
447447
}
448448

449449
func TestLoadInvalidConfigWithNonArrayDimensionClients(t *testing.T) {
@@ -512,7 +512,7 @@ func TestFilteringConfig(t *testing.T) {
512512
},
513513
},
514514
}, fsCfg)
515-
require.NoError(t, fsCfg.validate())
515+
require.NoError(t, fsCfg.Validate())
516516
}
517517

518518
func TestInvalidFilteringConfig(t *testing.T) {
@@ -539,7 +539,7 @@ func TestInvalidFilteringConfig(t *testing.T) {
539539
},
540540
}, fsCfg)
541541

542-
err = fsCfg.validate()
542+
err = fsCfg.Validate()
543543
require.Error(t, err)
544544
require.EqualError(t, err, "unexpected end of input")
545545
}
@@ -572,7 +572,7 @@ func TestLoadConfigWithNestedMonitorConfig(t *testing.T) {
572572
Timeout: timeutil.Duration(5 * time.Second),
573573
},
574574
}, telegrafExecCfg)
575-
require.NoError(t, telegrafExecCfg.validate())
575+
require.NoError(t, telegrafExecCfg.Validate())
576576

577577
cm, err = cfg.Sub(component.MustNewIDWithName(typeStr, "kubernetes_volumes").String())
578578
require.NoError(t, err)
@@ -597,5 +597,11 @@ func TestLoadConfigWithNestedMonitorConfig(t *testing.T) {
597597
},
598598
},
599599
}, k8sVolumesCfg)
600-
require.NoError(t, k8sVolumesCfg.validate())
600+
require.NoError(t, k8sVolumesCfg.Validate())
601+
}
602+
603+
func TestInvalidMonitorConfig(t *testing.T) {
604+
t.Cleanup(cleanUp())
605+
cfg := newConfig("cpu", -123)
606+
assert.EqualError(t, cfg.Validate(), "intervalSeconds must be greater than 0s (-123 provided)")
601607
}

‎pkg/receiver/smartagentreceiver/factory.go

-5
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,6 @@ func getOrCreateReceiver(cfg component.Config, params otelcolreceiver.Settings)
3939
defer receiverStoreLock.Unlock()
4040
receiverConfig := cfg.(*Config)
4141

42-
err := receiverConfig.validate()
43-
if err != nil {
44-
return nil, err
45-
}
46-
4742
receiverInst, ok := receiverStore[receiverConfig]
4843
if !ok {
4944
receiverInst = newReceiver(params, *receiverConfig)

‎pkg/receiver/smartagentreceiver/factory_test.go

-43
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020

2121
"github.com/signalfx/signalfx-agent/pkg/monitors/haproxy"
2222
"github.com/stretchr/testify/assert"
23-
"github.com/stretchr/testify/require"
2423
"go.opentelemetry.io/collector/component"
2524
"go.opentelemetry.io/collector/component/componenttest"
2625
"go.opentelemetry.io/collector/consumer/consumertest"
@@ -48,20 +47,6 @@ func TestCreateMetrics(t *testing.T) {
4847
assert.Same(t, receiver, receiverStore[cfg.(*Config)])
4948
}
5049

51-
func TestCreateMetricsWithInvalidConfig(t *testing.T) {
52-
factory := NewFactory()
53-
cfg := &Config{}
54-
require.Error(t, cfg.validate())
55-
56-
params := otelcolreceiver.Settings{ID: component.MustNewID(typeStr)}
57-
receiver, err := factory.CreateMetrics(context.Background(), params, cfg, consumertest.NewNop())
58-
require.Error(t, err)
59-
assert.EqualError(t, err, `you must specify a "type" for a smartagent receiver`)
60-
assert.Nil(t, receiver)
61-
62-
assert.NotContains(t, receiverStore, cfg)
63-
}
64-
6550
func TestCreateLogs(t *testing.T) {
6651
factory := NewFactory()
6752
cfg := factory.CreateDefaultConfig()
@@ -76,20 +61,6 @@ func TestCreateLogs(t *testing.T) {
7661
assert.Same(t, receiver, receiverStore[cfg.(*Config)])
7762
}
7863

79-
func TestCreateLogsWithInvalidConfig(t *testing.T) {
80-
factory := NewFactory()
81-
cfg := &Config{}
82-
require.Error(t, cfg.validate())
83-
84-
params := otelcolreceiver.Settings{ID: component.MustNewID(typeStr)}
85-
receiver, err := factory.CreateLogs(context.Background(), params, cfg, consumertest.NewNop())
86-
require.Error(t, err)
87-
assert.EqualError(t, err, `you must specify a "type" for a smartagent receiver`)
88-
assert.Nil(t, receiver)
89-
90-
assert.NotContains(t, receiverStore, cfg)
91-
}
92-
9364
func TestCreateTraces(t *testing.T) {
9465
factory := NewFactory()
9566
cfg := factory.CreateDefaultConfig()
@@ -104,20 +75,6 @@ func TestCreateTraces(t *testing.T) {
10475
assert.Same(t, receiver, receiverStore[cfg.(*Config)])
10576
}
10677

107-
func TestCreateTracesWithInvalidConfig(t *testing.T) {
108-
factory := NewFactory()
109-
cfg := &Config{}
110-
require.Error(t, cfg.validate())
111-
112-
params := otelcolreceiver.Settings{ID: component.MustNewID(typeStr)}
113-
receiver, err := factory.CreateTraces(context.Background(), params, cfg, consumertest.NewNop())
114-
require.Error(t, err)
115-
assert.EqualError(t, err, `you must specify a "type" for a smartagent receiver`)
116-
assert.Nil(t, receiver)
117-
118-
assert.NotContains(t, receiverStore, cfg)
119-
}
120-
12178
func TestCreateMetricsThenLogsAndThenTracesReceiver(t *testing.T) {
12279
factory := NewFactory()
12380
cfg := factory.CreateDefaultConfig()

‎pkg/receiver/smartagentreceiver/go.mod

+1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ require (
1717
go.opentelemetry.io/collector/component v1.31.0
1818
go.opentelemetry.io/collector/component/componenttest v0.125.0
1919
go.opentelemetry.io/collector/confmap v1.31.0
20+
go.opentelemetry.io/collector/confmap/xconfmap v0.125.0
2021
go.opentelemetry.io/collector/consumer v1.31.0
2122
go.opentelemetry.io/collector/consumer/consumertest v0.125.0
2223
go.opentelemetry.io/collector/exporter v0.125.0

‎pkg/receiver/smartagentreceiver/go.sum

+2
Original file line numberDiff line numberDiff line change
@@ -798,6 +798,8 @@ go.opentelemetry.io/collector/config/configretry v1.31.0 h1:GWl/UM7+xNCmXBz5lvaM
798798
go.opentelemetry.io/collector/config/configretry v1.31.0/go.mod h1:QNnb+MCk7aS1k2EuGJMtlNCltzD7b8uC7Xel0Dxm1wQ=
799799
go.opentelemetry.io/collector/confmap v1.31.0 h1:+AW5VJc1rCtgEyGd+1J5uSNw/kVZ98+lKO/pqXEwVvU=
800800
go.opentelemetry.io/collector/confmap v1.31.0/go.mod h1:TdutQlIoHDPXcZ2xZ0QWGRkSFC8oTKO61zTx569dvrY=
801+
go.opentelemetry.io/collector/confmap/xconfmap v0.125.0 h1:Y0LPtz+xgtRYVAk2gZmvnBROEJj8C3YDiFPj5URbsX8=
802+
go.opentelemetry.io/collector/confmap/xconfmap v0.125.0/go.mod h1:8hNqCMs9Gzahh4W1h5XWOrQ+bE6NfP13WAggNyExJJs=
801803
go.opentelemetry.io/collector/consumer v1.31.0 h1:L+y66ywxLHnAxnUxv0JDwUf5bFj53kMxCCyEfRKlM7s=
802804
go.opentelemetry.io/collector/consumer v1.31.0/go.mod h1:rPsqy5ni+c6xNMUkOChleZYO/nInVY6eaBNZ1FmWJVk=
803805
go.opentelemetry.io/collector/consumer/consumererror v0.125.0 h1:Qq9SgbxlJoRn0952dj4lPJhcuBiqKzD1aNxCfa+Bz00=

‎pkg/receiver/smartagentreceiver/receiver.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,6 @@ func (r *receiver) Start(_ context.Context, host component.Host) error {
9797
return nil
9898
}
9999

100-
err := r.config.validate()
101-
if err != nil {
102-
return fmt.Errorf("config validation failed for %q: %w", r.params.ID.String(), err)
103-
}
104-
105100
configCore := r.config.monitorConfig.MonitorConfigCore()
106101
monitorType := configCore.Type
107102
monitorID := nonWordCharacters.ReplaceAllString(r.params.ID.String(), "")
@@ -123,6 +118,7 @@ func (r *receiver) Start(_ context.Context, host component.Host) error {
123118
if !r.config.acceptsEndpoints {
124119
r.logger.Debug("This Smart Agent monitor does not use Host/Port config fields. If either are set, they will be ignored.", zap.String("monitor_type", monitorType))
125120
}
121+
var err error
126122
r.monitor, err = r.createMonitor(monitorType, host)
127123
if err != nil {
128124
return fmt.Errorf("failed creating monitor %q: %w", monitorType, err)

‎pkg/receiver/smartagentreceiver/receiver_test.go

-10
Original file line numberDiff line numberDiff line change
@@ -200,16 +200,6 @@ func TestStripMonitorTypePrefix(t *testing.T) {
200200
assert.Equal(t, "cpu", stripMonitorTypePrefix("cpu"))
201201
}
202202

203-
func TestStartReceiverWithInvalidMonitorConfig(t *testing.T) {
204-
t.Cleanup(cleanUp())
205-
cfg := newConfig("cpu", -123)
206-
receiver := newReceiver(newReceiverCreateSettings("invalid", t), cfg)
207-
err := receiver.Start(context.Background(), componenttest.NewNopHost())
208-
assert.EqualError(t, err,
209-
"config validation failed for \"smartagent/invalid\": intervalSeconds must be greater than 0s (-123 provided)",
210-
)
211-
}
212-
213203
func TestStartReceiverWithUnknownMonitorType(t *testing.T) {
214204
t.Cleanup(cleanUp())
215205
cfg := newConfig("notamonitortype", 1)

0 commit comments

Comments
 (0)