improve(go.d/snmp): optimize Check() to avoid heavy collection (#20813)
Ilya Mashchenko committed
Aug 12, 2025 at 22:54 UTC
1cf24370c18e203c2804ead93bd883504e760b37
6 files changed
+92
-127
src/go/plugin/go.d/collector/snmp/charts.go
+15
-12
@@ -165,7 +165,7 @@ func (c *Collector) addNetIfaceCharts(iface *netInterface) {
165
charts := netIfaceChartsTmpl.Copy()
166
167
for _, chart := range *charts {
168
- chart.ID = fmt.Sprintf(chart.ID, cleanIfaceName(iface.ifName))
168
+ chart.ID = fmt.Sprintf(chart.ID, cleanMetricName.Replace(iface.ifName))
169
chart.Labels = []module.Label{
170
{Key: "vendor", Value: c.sysInfo.Organization},
171
{Key: "sysName", Value: c.sysInfo.Name},
@@ -184,7 +184,7 @@ func (c *Collector) addNetIfaceCharts(iface *netInterface) {
184
}
185
186
func (c *Collector) removeNetIfaceCharts(iface *netInterface) {
187
- px := fmt.Sprintf("snmp_device_net_iface_%s_", cleanIfaceName(iface.ifName))
187
+ px := fmt.Sprintf("snmp_device_net_iface_%s_", cleanMetricName.Replace(iface.ifName))
188
for _, chart := range *c.Charts() {
189
if strings.HasPrefix(chart.ID, px) {
190
chart.MarkRemove()
@@ -315,13 +315,12 @@ func (c *Collector) addProfileScalarMetricChart(m ddsnmp.Metric) {
315
return
316
}
317
318
- r := strings.NewReplacer(".", "_", " ", "_")
318
chart := &module.Chart{
320
- ID: fmt.Sprintf("snmp_device_prof_%s", r.Replace(m.Name)),
319
+ ID: fmt.Sprintf("snmp_device_prof_%s", cleanMetricName.Replace(m.Name)),
320
Title: m.Description,
321
Units: m.Unit,
322
Fam: m.Family,
324
- Ctx: fmt.Sprintf("snmp.device_prof_%s", r.Replace(m.Name)),
323
+ Ctx: fmt.Sprintf("snmp.device_prof_%s", cleanMetricName.Replace(m.Name)),
324
Priority: prioProfileChart,
325
}
326
if chart.Title == "" {
@@ -375,13 +374,12 @@ func (c *Collector) addProfileTableMetricChart(m ddsnmp.Metric) {
374
375
key := tableMetricKey(m)
376
378
- r := strings.NewReplacer(".", "_", " ", "_")
377
chart := &module.Chart{
380
- ID: fmt.Sprintf("snmp_device_prof_%s", r.Replace(key)),
378
+ ID: fmt.Sprintf("snmp_device_prof_%s", cleanMetricName.Replace(key)),
379
Title: m.Description,
380
Units: m.Unit,
381
Fam: m.Family,
384
- Ctx: fmt.Sprintf("snmp.device_prof_%s", r.Replace(m.Name)),
382
+ Ctx: fmt.Sprintf("snmp.device_prof_%s", cleanMetricName.Replace(m.Name)),
383
Priority: prioProfileChart,
384
}
385
if chart.Title == "" {
@@ -432,6 +430,14 @@ func (c *Collector) addProfileTableMetricChart(m ddsnmp.Metric) {
430
}
431
}
432
433
+func (c *Collector) removeProfileTableMetricChart(key string) {
434
+ id := fmt.Sprintf("snmp_device_prof_%s", cleanMetricName.Replace(key))
435
+ if chart := c.Charts().Get(id); chart != nil {
436
+ chart.MarkRemove()
437
+ chart.MarkNotCreated()
438
+ }
439
+}
440
+
441
func dimAlgoFromDdSnmpType(m ddsnmp.Metric) module.DimAlgo {
442
switch m.MetricType {
443
case ddprofiledefinition.ProfileMetricTypeGauge,
@@ -443,7 +449,4 @@ func dimAlgoFromDdSnmpType(m ddsnmp.Metric) module.DimAlgo {
449
}
450
}
451
446
-func cleanIfaceName(name string) string {
447
- r := strings.NewReplacer(".", "_", " ", "_")
448
- return r.Replace(name)
449
-}
452
+var cleanMetricName = strings.NewReplacer(".", "_", " ", "_")
src/go/plugin/go.d/collector/snmp/collect.go
+48
-3
@@ -35,7 +35,7 @@ func (c *Collector) collect() (map[string]int64, error) {
35
36
if c.ddSnmpColl == nil {
37
c.ddSnmpColl = ddsnmpcollector.New(c.snmpClient, c.snmpProfiles, c.Logger, si.SysObjectID)
38
- c.ddSnmpColl.DoTableMetrics = c.EnableProfilesTableMetrics
38
+ c.ddSnmpColl.DoTableMetrics = c.EnableProfilesTableMetrics && c.snmpBulkWalkOk
39
}
40
41
if c.CreateVnode {
@@ -64,12 +64,11 @@ func (c *Collector) collect() (map[string]int64, error) {
64
return nil, err
65
}
66
67
- if c.collectIfMib {
67
+ if c.snmpBulkWalkOk && c.collectIfMib {
68
if err := c.collectNetworkInterfaces(mx); err != nil {
69
return nil, err
70
}
71
}
72
-
72
if len(c.customOids) > 0 {
73
if err := c.collectOIDs(mx); err != nil {
74
return nil, err
@@ -189,6 +188,52 @@ func (c *Collector) setupProfiles(sysObjectID string) []*ddsnmp.Profile {
188
return snmpProfiles
189
}
190
191
+func (c *Collector) adjustMaxRepetitions() (bool, error) {
192
+ orig := c.Config.Options.MaxRepetitions
193
+ maxReps := c.Config.Options.MaxRepetitions
194
+ attempts := 0
195
+ const maxAttempts = 20 // Prevent infinite loops
196
+
197
+ for maxReps > 0 && attempts < maxAttempts {
198
+ attempts++
199
+
200
+ v, err := c.walkAll(snmpsd.RootOidMibSystem)
201
+ if err != nil {
202
+ return false, err
203
+ }
204
+
205
+ if len(v) > 0 {
206
+ //c.Config.Options.MaxRepetitions = maxReps
207
+ if orig != maxReps {
208
+ c.Infof("adjusted max_repetitions: %d → %d (took %d attempts)", orig, maxReps, attempts)
209
+ }
210
+ return true, nil
211
+ }
212
+
213
+ // Adaptive decrease strategy
214
+ prevMaxReps := maxReps
215
+ if maxReps > 50 {
216
+ maxReps -= 10
217
+ } else if maxReps > 10 {
218
+ maxReps -= 5
219
+ } else if maxReps > 5 {
220
+ maxReps -= 2
221
+ } else {
222
+ maxReps--
223
+ }
224
+
225
+ maxReps = max(0, maxReps) // Ensure non-negative
226
+
227
+ c.Debugf("max_repetitions=%d returned no data, trying %d", prevMaxReps, maxReps)
228
+ c.snmpClient.SetMaxRepetitions(uint32(maxReps))
229
+ }
230
+
231
+ // Restore original value since nothing worked
232
+ c.snmpClient.SetMaxRepetitions(uint32(orig))
233
+ c.Debugf("unable to find working max_repetitions value after %d attempts", attempts)
234
+ return false, nil
235
+}
236
+
237
func pduToInt(pdu gosnmp.SnmpPDU) (int64, error) {
238
switch pdu.Type {
239
case gosnmp.Counter32, gosnmp.Counter64, gosnmp.Integer, gosnmp.Gauge32, gosnmp.TimeTicks:
src/go/plugin/go.d/collector/snmp/collect_if_mib.go
+1
-54
@@ -3,7 +3,6 @@
3
package snmp
4
5
import (
6
- "errors"
6
"fmt"
7
"log/slog"
8
"sort"
@@ -21,26 +20,6 @@ const (
20
)
21
22
func (c *Collector) collectNetworkInterfaces(mx map[string]int64) error {
24
- if c.checkMaxReps {
25
- ok, err := c.adjustMaxRepetitions()
26
- if err != nil {
27
- return err
28
- }
29
-
30
- c.checkMaxReps = false
31
-
32
- if !ok {
33
- c.collectIfMib = false
34
-
35
- if len(c.customOids) == 0 {
36
- return errors.New("no IF-MIB data returned")
37
- }
38
-
39
- c.Warning("no IF-MIB data returned")
40
- return nil
41
- }
42
- }
43
-
23
ifMibTable, err := c.walkAll(rootOidIfMibIfTable)
24
if err != nil {
25
return err
@@ -141,7 +120,7 @@ func (c *Collector) collectNetworkInterfaces(mx map[string]int64) error {
120
case oidIfHCOutMulticastPkts:
121
iface.ifHCOutMulticastPkts, err = pduToInt(pdu)
122
case oidIfHCOutBroadcastPkts:
144
- iface.ifHCOutMulticastPkts, err = pduToInt(pdu)
123
+ iface.ifHCOutBroadcastPkts, err = pduToInt(pdu)
124
case oidIfHighSpeed:
125
iface.ifHighSpeed, err = pduToInt(pdu)
126
case oidIfAlias:
@@ -238,35 +217,3 @@ func (c *Collector) collectNetworkInterfaces(mx map[string]int64) error {
217
218
return nil
219
}
241
-
242
-func (c *Collector) adjustMaxRepetitions() (bool, error) {
243
- orig := c.Config.Options.MaxRepetitions
244
- maxReps := c.Config.Options.MaxRepetitions
245
-
246
- for {
247
- v, err := c.walkAll(oidIfIndex)
248
- if err != nil {
249
- return false, err
250
- }
251
-
252
- if len(v) > 0 {
253
- if orig != maxReps {
254
- c.Infof("changed 'max_repetitions' %d => %d", orig, maxReps)
255
- }
256
- return true, nil
257
- }
258
-
259
- if maxReps > 5 {
260
- maxReps = max(5, maxReps-5)
261
- } else {
262
- maxReps--
263
- }
264
-
265
- if maxReps <= 0 {
266
- return false, nil
267
- }
268
-
269
- c.Debugf("no IF-MIB data returned, trying to decrese 'max_repetitions' to %d", maxReps)
270
- c.snmpClient.SetMaxRepetitions(uint32(maxReps))
271
- }
272
-}
src/go/plugin/go.d/collector/snmp/collect_profiles.go
+1
@@ -84,6 +84,7 @@ func (c *Collector) collectProfileTableMetrics(mx map[string]int64, pms []*ddsnm
84
for key := range c.seenTableMetrics {
85
if !seen[key] {
86
delete(c.seenTableMetrics, key)
87
+ c.removeProfileTableMetricChart(key)
88
}
89
}
90
}
src/go/plugin/go.d/collector/snmp/collector.go
+12
-10
@@ -5,7 +5,6 @@ package snmp
5
import (
6
"context"
7
_ "embed"
8
- "errors"
8
"fmt"
9
10
"github.com/gosnmp/gosnmp"
@@ -60,9 +59,9 @@ func New() *Collector {
59
60
newSnmpClient: gosnmp.NewHandler,
61
63
- checkMaxReps: true,
64
- collectIfMib: true,
65
- netInterfaces: make(map[string]*netInterface),
62
+ snmpBulkWalkOk: true,
63
+ netInterfaces: make(map[string]*netInterface),
64
+ collectIfMib: true,
65
66
seenScalarMetrics: make(map[string]bool),
67
seenTableMetrics: make(map[string]bool),
@@ -85,8 +84,8 @@ type Collector struct {
84
netIfaceFilterByName matcher.Matcher
85
netIfaceFilterByType matcher.Matcher
86
88
- checkMaxReps bool
89
- collectIfMib bool
87
+ snmpBulkWalkOk bool
88
+ collectIfMib bool // only for tests
89
90
netInterfaces map[string]*netInterface
91
@@ -140,14 +139,17 @@ func (c *Collector) Init(context.Context) error {
139
}
140
141
func (c *Collector) Check(context.Context) error {
143
- mx, err := c.collect()
142
+ if _, err := snmpsd.GetSysInfo(c.snmpClient); err != nil {
143
+ return err
144
+ }
145
+ ok, err := c.adjustMaxRepetitions()
146
if err != nil {
147
return err
148
}
147
-
148
- if len(mx) == 0 {
149
- return errors.New("no metrics collected")
149
+ if !ok {
150
+ c.Warningf("SNMP bulk walk disabled: table metrics collection unavailable (device may not support GETBULK or max-repetitions adjustment failed)")
151
}
152
+ c.snmpBulkWalkOk = ok
153
154
return nil
155
}
src/go/plugin/go.d/collector/snmp/collector_test.go
+15
-48
@@ -159,7 +159,7 @@ func TestCollector_Charts(t *testing.T) {
159
if collr.EnableProfiles {
160
setMockClientSysObjectidExpect(m)
161
}
162
- setMockClientSysExpect(m)
162
+ setMockClientSysinfoAndUptimeExpect(m)
163
setMockClientIfMibExpect(m)
164
165
return collr
@@ -190,6 +190,7 @@ func TestCollector_Charts(t *testing.T) {
190
require.NoError(t, collr.Init(context.Background()))
191
192
if test.doCollect {
193
+ _ = collr.Check(context.Background())
194
_ = collr.Collect(context.Background())
195
}
196
@@ -203,42 +204,13 @@ func TestCollector_Check(t *testing.T) {
204
wantFail bool
205
prepareSNMP func(m *snmpmock.MockHandler) *Collector
206
}{
206
- "success when collecting IF-MIB": {
207
+ "success when sysinfo collected": {
208
wantFail: false,
209
prepareSNMP: func(m *snmpmock.MockHandler) *Collector {
210
collr := New()
211
collr.Config = prepareV2Config()
211
- if collr.EnableProfiles {
212
- setMockClientSysObjectidExpect(m)
213
- }
214
- setMockClientIfMibExpect(m)
212
216
- return collr
217
- },
218
- },
219
- "success only custom OIDs supported type": {
220
- wantFail: false,
221
- prepareSNMP: func(m *snmpmock.MockHandler) *Collector {
222
- collr := New()
223
- collr.Config = prepareConfigWithUserCharts(prepareV2Config(), 0, 3)
224
- collr.collectIfMib = false
225
-
226
- if collr.EnableProfiles {
227
- setMockClientSysObjectidExpect(m)
228
- }
229
-
230
- m.EXPECT().Get(gomock.Any()).Return(&gosnmp.SnmpPacket{
231
- Variables: []gosnmp.SnmpPDU{
232
- {Value: 10, Type: gosnmp.Counter32},
233
- {Value: 20, Type: gosnmp.Counter64},
234
- {Value: 30, Type: gosnmp.Gauge32},
235
- {Value: 1, Type: gosnmp.Boolean},
236
- {Value: 40, Type: gosnmp.Gauge32},
237
- {Value: 50, Type: gosnmp.TimeTicks},
238
- {Value: 60, Type: gosnmp.Uinteger32},
239
- {Value: 70, Type: gosnmp.Integer},
240
- },
241
- }, nil).Times(1)
213
+ setMockClientSysInfoExpect(m)
214
215
return collr
216
},
@@ -249,10 +221,7 @@ func TestCollector_Check(t *testing.T) {
221
collr := New()
222
collr.Config = prepareConfigWithUserCharts(prepareV2Config(), 0, 3)
223
collr.collectIfMib = false
252
- if collr.EnableProfiles {
253
- setMockClientSysObjectidExpect(m)
254
- }
255
- m.EXPECT().Get(gomock.Any()).Return(nil, errors.New("mock Get() error")).Times(1)
224
+ m.EXPECT().WalkAll(snmpsd.RootOidMibSystem).Return(nil, errors.New("mock Get() error")).Times(1)
225
226
return collr
227
},
@@ -265,7 +234,6 @@ func TestCollector_Check(t *testing.T) {
234
defer cleanup()
235
236
setMockClientInitExpect(mockSNMP)
268
- setMockClientSysExpect(mockSNMP)
237
238
collr := test.prepareSNMP(mockSNMP)
239
collr.newSnmpClient = func() gosnmp.Handler { return mockSNMP }
@@ -299,7 +267,6 @@ func TestCollector_Collect(t *testing.T) {
267
return collr
268
},
269
wantCollected: map[string]int64{
302
- //"TestMetric": 1,
270
"net_iface_ether1_admin_status_down": 0,
271
"net_iface_ether1_admin_status_testing": 0,
272
"net_iface_ether1_admin_status_up": 1,
@@ -326,13 +293,13 @@ func TestCollector_Collect(t *testing.T) {
293
"net_iface_ether2_admin_status_testing": 0,
294
"net_iface_ether2_admin_status_up": 1,
295
"net_iface_ether2_bcast_in": 0,
329
- "net_iface_ether2_bcast_out": 0,
296
+ "net_iface_ether2_bcast_out": 7386,
297
"net_iface_ether2_discards_in": 0,
298
"net_iface_ether2_discards_out": 0,
299
"net_iface_ether2_errors_in": 0,
300
"net_iface_ether2_errors_out": 0,
301
"net_iface_ether2_mcast_in": 1891,
335
- "net_iface_ether2_mcast_out": 7386,
302
+ "net_iface_ether2_mcast_out": 28844,
303
"net_iface_ether2_oper_status_dormant": 0,
304
"net_iface_ether2_oper_status_down": 0,
305
"net_iface_ether2_oper_status_lowerLayerDown": 0,
@@ -496,13 +463,15 @@ func TestCollector_Collect(t *testing.T) {
463
defer cleanup()
464
465
setMockClientInitExpect(mockSNMP)
499
- setMockClientSysExpect(mockSNMP)
466
+ setMockClientSysinfoAndUptimeExpect(mockSNMP)
467
468
collr := test.prepareSNMP(mockSNMP)
469
collr.newSnmpClient = func() gosnmp.Handler { return mockSNMP }
470
471
require.NoError(t, collr.Init(context.Background()))
472
473
+ _ = collr.Check(context.Background())
474
+
475
mx := collr.Collect(context.Background())
476
477
if collr.EnableProfiles {
@@ -632,7 +601,7 @@ func setMockClientSysObjectidExpect(m *snmpmock.MockHandler) {
601
602
}
603
635
-func setMockClientSysExpect(m *snmpmock.MockHandler) {
604
+func setMockClientSysInfoExpect(m *snmpmock.MockHandler) {
605
m.EXPECT().WalkAll(snmpsd.RootOidMibSystem).Return([]gosnmp.SnmpPDU{
606
{Name: snmpsd.OidSysDescr, Value: []uint8("mock sysDescr"), Type: gosnmp.OctetString},
607
{Name: snmpsd.OidSysObject, Value: ".1.3.6.1.4.1.14988.1", Type: gosnmp.ObjectIdentifier},
@@ -640,6 +609,10 @@ func setMockClientSysExpect(m *snmpmock.MockHandler) {
609
{Name: snmpsd.OidSysName, Value: []uint8("mock sysName"), Type: gosnmp.OctetString},
610
{Name: snmpsd.OidSysLocation, Value: []uint8("mock sysLocation"), Type: gosnmp.OctetString},
611
}, nil).MinTimes(1)
612
+}
613
+
614
+func setMockClientSysinfoAndUptimeExpect(m *snmpmock.MockHandler) {
615
+ setMockClientSysInfoExpect(m)
616
617
m.EXPECT().Get([]string{snmpsd.OidSysUptime}).Return(&gosnmp.SnmpPacket{
618
Variables: []gosnmp.SnmpPDU{
@@ -649,12 +622,6 @@ func setMockClientSysExpect(m *snmpmock.MockHandler) {
622
}
623
624
func setMockClientIfMibExpect(m *snmpmock.MockHandler) {
652
- m.EXPECT().WalkAll(oidIfIndex).Return([]gosnmp.SnmpPDU{
653
- {Name: oidIfIndex + ".1", Value: 1, Type: gosnmp.Integer},
654
- {Name: oidIfIndex + ".2", Value: 2, Type: gosnmp.Integer},
655
- {Name: oidIfIndex + ".17", Value: 17, Type: gosnmp.Integer},
656
- {Name: oidIfIndex + ".18", Value: 18, Type: gosnmp.Integer},
657
- }, nil).MinTimes(1)
625
m.EXPECT().WalkAll(rootOidIfMibIfTable).Return([]gosnmp.SnmpPDU{
626
{Name: oidIfIndex + ".1", Value: 1, Type: gosnmp.Integer},
627
{Name: oidIfIndex + ".2", Value: 2, Type: gosnmp.Integer},