chore(go.d/ddsnmp): split table collection into walk and process phases (#20479)
Ilya Mashchenko committed
Jun 13, 2025 at 16:06 UTC
1d81615daa02fd267a8b62d68ceba5c52ad38123
2 files changed
+373
-34
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collect_table.go
+174
-34
@@ -14,8 +14,23 @@ import (
14
"github.com/netdata/netdata/go/plugins/plugin/go.d/collector/snmp/ddsnmp/ddprofiledefinition"
15
)
16
17
+type tableWalkResult struct {
18
+ tableOID string
19
+ pdus map[string]gosnmp.SnmpPDU
20
+ config ddprofiledefinition.MetricsConfig
21
+}
22
+
23
func (c *Collector) collectTableMetrics(prof *ddsnmp.Profile) ([]Metric, error) {
18
- var metrics []Metric
24
+ walkResults, err := c.walkAllTables(prof)
25
+ if err != nil {
26
+ return nil, err
27
+ }
28
+
29
+ return c.processTableWalkResults(walkResults)
30
+}
31
+
32
+func (c *Collector) walkAllTables(prof *ddsnmp.Profile) ([]tableWalkResult, error) {
33
+ var results []tableWalkResult
34
var errs []error
35
var missingOIDs []string
36
@@ -32,66 +47,94 @@ func (c *Collector) collectTableMetrics(prof *ddsnmp.Profile) ([]Metric, error)
47
}
48
49
doneOids[cfg.Table.OID] = true
35
- tableMetrics, err := c.collectSingleTable(cfg)
50
+
51
+ // Check if we should skip this table
52
+ skipTable := false
53
+
54
+ for _, tagCfg := range cfg.MetricTags {
55
+ if tagCfg.Table != "" && tagCfg.Table != cfg.Table.Name {
56
+ c.log.Debugf("Skipping table %s: has cross-table tag from %s", cfg.Table.Name, tagCfg.Table)
57
+ skipTable = true
58
+ break
59
+ }
60
+ if len(tagCfg.IndexTransform) > 0 {
61
+ c.log.Debugf("Skipping table %s: has index transformation", cfg.Table.Name)
62
+ skipTable = true
63
+ break
64
+ }
65
+ }
66
+
67
+ if skipTable {
68
+ continue
69
+ }
70
+
71
+ pdus, err := c.snmpWalk(cfg.Table.OID)
72
if err != nil {
37
- errs = append(errs, fmt.Errorf("table '%s': %w", cfg.Table.Name, err))
73
+ errs = append(errs, fmt.Errorf("failed to walk table '%s': %w", cfg.Table.Name, err))
74
continue
75
}
40
- metrics = append(metrics, tableMetrics...)
76
+
77
+ if len(pdus) > 0 {
78
+ results = append(results, tableWalkResult{
79
+ tableOID: cfg.Table.OID,
80
+ pdus: pdus,
81
+ config: cfg,
82
+ })
83
+ }
84
}
85
86
if len(missingOIDs) > 0 {
87
c.log.Debugf("table metrics missing OIDs: %v", missingOIDs)
88
}
89
47
- if len(metrics) == 0 && len(errs) > 0 {
90
+ if len(results) == 0 && len(errs) > 0 {
91
return nil, errors.Join(errs...)
92
}
93
51
- return metrics, nil
94
+ return results, nil
95
}
96
54
-func (c *Collector) collectSingleTable(cfg ddprofiledefinition.MetricsConfig) ([]Metric, error) {
55
- for _, tagCfg := range cfg.MetricTags {
56
- if tagCfg.Table != "" && tagCfg.Table != cfg.Table.Name {
57
- c.log.Debugf("Skipping table %s: has cross-table tag from %s", cfg.Table.Name, tagCfg.Table)
58
- return nil, nil
59
- }
60
- if len(tagCfg.IndexTransform) > 0 {
61
- c.log.Debugf("Skipping table %s: has index transformation", cfg.Table.Name)
62
- return nil, nil
63
- }
64
- }
97
+func (c *Collector) processTableWalkResults(walkResults []tableWalkResult) ([]Metric, error) {
98
+ var metrics []Metric
99
+ var errs []error
100
66
- columnOIDs := make(map[string]ddprofiledefinition.SymbolConfig)
67
- for _, sym := range cfg.Symbols {
68
- columnOIDs[trimOID(sym.OID)] = sym
101
+ // Build a map for quick lookup of walked data by table OID
102
+ walkedData := make(map[string]map[string]gosnmp.SnmpPDU)
103
+ for _, result := range walkResults {
104
+ walkedData[result.tableOID] = result.pdus
105
}
106
71
- tagColumnOIDs := make(map[string]ddprofiledefinition.MetricTagConfig)
72
- for _, tagCfg := range cfg.MetricTags {
73
- if tagCfg.Table == "" || tagCfg.Table == cfg.Table.Name {
74
- tagColumnOIDs[trimOID(tagCfg.Symbol.OID)] = tagCfg
107
+ // Process each table's walked data
108
+ for _, result := range walkResults {
109
+ tableMetrics, err := c.processTableData(result.config, result.pdus, walkedData)
110
+ if err != nil {
111
+ errs = append(errs, fmt.Errorf("table '%s': %w", result.config.Table.Name, err))
112
+ continue
113
}
114
+ metrics = append(metrics, tableMetrics...)
115
+ }
116
+
117
+ if len(metrics) == 0 && len(errs) > 0 {
118
+ return nil, errors.Join(errs...)
119
}
120
121
+ return metrics, nil
122
+}
123
+
124
+func (c *Collector) processTableData(cfg ddprofiledefinition.MetricsConfig, pdus map[string]gosnmp.SnmpPDU, allWalkedData map[string]map[string]gosnmp.SnmpPDU) ([]Metric, error) {
125
+ // Try to use cache if available
126
if cachedOIDs, cachedTags, ok := c.tableCache.getCachedData(cfg.Table.OID); ok {
79
- metrics, err := c.collectTableWithCache(cfg, cachedOIDs, cachedTags, columnOIDs)
127
+ metrics, err := c.collectTableWithCache(cfg, cachedOIDs, cachedTags, buildColumnOIDs(cfg))
128
if err == nil {
129
c.log.Debugf("Successfully collected table %s using cache", cfg.Table.Name)
130
return metrics, nil
131
}
84
- c.log.Debugf("Cached collection failed for table %s, falling back to walk: %v", cfg.Table.Name, err)
85
- }
86
-
87
- pdus, err := c.snmpWalk(cfg.Table.OID)
88
- if err != nil {
89
- return nil, fmt.Errorf("failed to walk table: %w", err)
132
+ c.log.Debugf("Cached collection failed for table %s, falling back to process walked data: %v", cfg.Table.Name, err)
133
}
134
92
- if len(pdus) == 0 {
93
- return nil, nil
94
- }
135
+ // Process without cache
136
+ columnOIDs := buildColumnOIDs(cfg)
137
+ tagColumnOIDs := buildTagColumnOIDs(cfg)
138
139
allColumnOIDs := make([]string, 0, len(columnOIDs)+len(tagColumnOIDs))
140
for oid := range columnOIDs {
@@ -135,6 +178,7 @@ func (c *Collector) collectSingleTable(cfg ddprofiledefinition.MetricsConfig) ([
178
for index, rowPDUs := range rows {
179
rowTags := make(map[string]string)
180
181
+ // Process tags for this row
182
for columnOID, tagCfg := range tagColumnOIDs {
183
pdu, ok := rowPDUs[columnOID]
184
if !ok {
@@ -153,6 +197,7 @@ func (c *Collector) collectSingleTable(cfg ddprofiledefinition.MetricsConfig) ([
197
}
198
}
199
200
+ // Process metrics for this row
201
for columnOID, sym := range columnOIDs {
202
pdu, ok := rowPDUs[columnOID]
203
if !ok {
@@ -182,12 +227,31 @@ func (c *Collector) collectSingleTable(cfg ddprofiledefinition.MetricsConfig) ([
227
}
228
}
229
230
+ // Cache the processed data
231
c.tableCache.cacheData(cfg.Table.OID, oidCache, tagCache)
232
c.log.Debugf("Cached table %s structure with %d rows", cfg.Table.Name, len(oidCache))
233
234
return metrics, nil
235
}
236
237
+func buildColumnOIDs(cfg ddprofiledefinition.MetricsConfig) map[string]ddprofiledefinition.SymbolConfig {
238
+ columnOIDs := make(map[string]ddprofiledefinition.SymbolConfig)
239
+ for _, sym := range cfg.Symbols {
240
+ columnOIDs[trimOID(sym.OID)] = sym
241
+ }
242
+ return columnOIDs
243
+}
244
+
245
+func buildTagColumnOIDs(cfg ddprofiledefinition.MetricsConfig) map[string]ddprofiledefinition.MetricTagConfig {
246
+ tagColumnOIDs := make(map[string]ddprofiledefinition.MetricTagConfig)
247
+ for _, tagCfg := range cfg.MetricTags {
248
+ if tagCfg.Table == "" || tagCfg.Table == cfg.Table.Name {
249
+ tagColumnOIDs[trimOID(tagCfg.Symbol.OID)] = tagCfg
250
+ }
251
+ }
252
+ return tagColumnOIDs
253
+}
254
+
255
func (c *Collector) collectTableWithCache(
256
cfg ddprofiledefinition.MetricsConfig,
257
cachedOIDs map[string]map[string]string,
@@ -336,3 +400,79 @@ func (c *Collector) snmpWalk(oid string) (map[string]gosnmp.SnmpPDU, error) {
400
401
return pdus, nil
402
}
403
+
404
+func (c *Collector) analyzeTableDependencies(prof *ddsnmp.Profile) map[string][]string {
405
+ deps := make(map[string][]string)
406
+
407
+ // Build a map of table name to OID for quick lookup
408
+ tableNameToOID := make(map[string]string)
409
+ for _, cfg := range prof.Definition.Metrics {
410
+ if cfg.Table.OID != "" {
411
+ tableNameToOID[cfg.Table.Name] = cfg.Table.OID
412
+ }
413
+ }
414
+
415
+ // Analyze each metric configuration
416
+ for _, cfg := range prof.Definition.Metrics {
417
+ if cfg.Table.OID == "" {
418
+ continue
419
+ }
420
+
421
+ mainTableOID := cfg.Table.OID
422
+ seenDeps := make(map[string]bool)
423
+
424
+ // Find all tables referenced in metric tags
425
+ for _, tagCfg := range cfg.MetricTags {
426
+ // Check if this tag references a different table
427
+ if tagCfg.Table != "" && tagCfg.Table != cfg.Table.Name {
428
+ // Skip if uses index transformation (Phase 5)
429
+ if len(tagCfg.IndexTransform) > 0 {
430
+ c.log.Debugf("Table %s has cross-table tag with index transformation from %s (not supported yet)",
431
+ cfg.Table.Name, tagCfg.Table)
432
+ continue
433
+ }
434
+
435
+ // Find the OID for the referenced table
436
+ if refTableOID, ok := tableNameToOID[tagCfg.Table]; ok {
437
+ if !seenDeps[refTableOID] {
438
+ deps[mainTableOID] = append(deps[mainTableOID], refTableOID)
439
+ seenDeps[refTableOID] = true
440
+ }
441
+ } else {
442
+ c.log.Debugf("Table %s references unknown table %s in metric tags",
443
+ cfg.Table.Name, tagCfg.Table)
444
+ }
445
+ }
446
+ }
447
+ }
448
+
449
+ // Log the dependencies for debugging
450
+ for tableOID, depList := range deps {
451
+ if len(depList) > 0 {
452
+ c.log.Debugf("Table %s depends on tables: %v", tableOID, depList)
453
+ }
454
+ }
455
+
456
+ return deps
457
+}
458
+
459
+// findTableOIDByName searches through the profile to find a table's OID given its name
460
+func (c *Collector) findTableOIDByName(prof *ddsnmp.Profile, tableName string) string {
461
+ for _, cfg := range prof.Definition.Metrics {
462
+ if cfg.Table.Name == tableName {
463
+ return cfg.Table.OID
464
+ }
465
+ }
466
+ return ""
467
+}
468
+
469
+// getConfigsForTable returns all metric configs that define metrics for a given table OID
470
+func (c *Collector) getConfigsForTable(prof *ddsnmp.Profile, tableOID string) []ddprofiledefinition.MetricsConfig {
471
+ var configs []ddprofiledefinition.MetricsConfig
472
+ for _, cfg := range prof.Definition.Metrics {
473
+ if cfg.Table.OID == tableOID {
474
+ configs = append(configs, cfg)
475
+ }
476
+ }
477
+ return configs
478
+}
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_test.go
+199
@@ -1364,6 +1364,205 @@ func TestCollector_Collect(t *testing.T) {
1364
}
1365
}
1366
1367
+func TestAnalyzeTableDependencies(t *testing.T) {
1368
+ tests := map[string]struct {
1369
+ profile *ddsnmp.Profile
1370
+ expected map[string][]string
1371
+ }{
1372
+ "simple cross-table reference": {
1373
+ profile: &ddsnmp.Profile{
1374
+ Definition: &ddprofiledefinition.ProfileDefinition{
1375
+ Metrics: []ddprofiledefinition.MetricsConfig{
1376
+ {
1377
+ MIB: "CISCO-IF-EXTENSION-MIB",
1378
+ Table: ddprofiledefinition.SymbolConfig{
1379
+ OID: "1.3.6.1.4.1.9.9.276.1.1.2",
1380
+ Name: "cieIfInterfaceTable",
1381
+ },
1382
+ Symbols: []ddprofiledefinition.SymbolConfig{
1383
+ {OID: "1.3.6.1.4.1.9.9.276.1.1.2.1.1", Name: "cieIfResetCount"},
1384
+ },
1385
+ MetricTags: []ddprofiledefinition.MetricTagConfig{
1386
+ {
1387
+ Symbol: ddprofiledefinition.SymbolConfigCompat{
1388
+ OID: "1.3.6.1.2.1.31.1.1.1.1",
1389
+ Name: "ifName",
1390
+ },
1391
+ Table: "ifXTable",
1392
+ Tag: "interface",
1393
+ },
1394
+ },
1395
+ },
1396
+ {
1397
+ MIB: "IF-MIB",
1398
+ Table: ddprofiledefinition.SymbolConfig{
1399
+ OID: "1.3.6.1.2.1.31.1.1",
1400
+ Name: "ifXTable",
1401
+ },
1402
+ Symbols: []ddprofiledefinition.SymbolConfig{
1403
+ {OID: "1.3.6.1.2.1.31.1.1.1.18", Name: "ifAlias"},
1404
+ },
1405
+ },
1406
+ },
1407
+ },
1408
+ },
1409
+ expected: map[string][]string{
1410
+ "1.3.6.1.4.1.9.9.276.1.1.2": {"1.3.6.1.2.1.31.1.1"},
1411
+ },
1412
+ },
1413
+ "multiple dependencies": {
1414
+ profile: &ddsnmp.Profile{
1415
+ Definition: &ddprofiledefinition.ProfileDefinition{
1416
+ Metrics: []ddprofiledefinition.MetricsConfig{
1417
+ {
1418
+ MIB: "MY-MIB",
1419
+ Table: ddprofiledefinition.SymbolConfig{
1420
+ OID: "1.3.6.1.4.1.1000.1",
1421
+ Name: "myTable",
1422
+ },
1423
+ Symbols: []ddprofiledefinition.SymbolConfig{
1424
+ {OID: "1.3.6.1.4.1.1000.1.1.1", Name: "myMetric"},
1425
+ },
1426
+ MetricTags: []ddprofiledefinition.MetricTagConfig{
1427
+ {
1428
+ Symbol: ddprofiledefinition.SymbolConfigCompat{
1429
+ OID: "1.3.6.1.2.1.31.1.1.1.1",
1430
+ Name: "ifName",
1431
+ },
1432
+ Table: "ifXTable",
1433
+ Tag: "interface",
1434
+ },
1435
+ {
1436
+ Symbol: ddprofiledefinition.SymbolConfigCompat{
1437
+ OID: "1.3.6.1.4.1.2000.1.1.1",
1438
+ Name: "otherName",
1439
+ },
1440
+ Table: "otherTable",
1441
+ Tag: "other_name",
1442
+ },
1443
+ },
1444
+ },
1445
+ {
1446
+ MIB: "IF-MIB",
1447
+ Table: ddprofiledefinition.SymbolConfig{
1448
+ OID: "1.3.6.1.2.1.31.1.1",
1449
+ Name: "ifXTable",
1450
+ },
1451
+ Symbols: []ddprofiledefinition.SymbolConfig{
1452
+ {OID: "1.3.6.1.2.1.31.1.1.1.18", Name: "ifAlias"},
1453
+ },
1454
+ },
1455
+ {
1456
+ MIB: "OTHER-MIB",
1457
+ Table: ddprofiledefinition.SymbolConfig{
1458
+ OID: "1.3.6.1.4.1.2000.1",
1459
+ Name: "otherTable",
1460
+ },
1461
+ Symbols: []ddprofiledefinition.SymbolConfig{
1462
+ {OID: "1.3.6.1.4.1.2000.1.1.2", Name: "otherMetric"},
1463
+ },
1464
+ },
1465
+ },
1466
+ },
1467
+ },
1468
+ expected: map[string][]string{
1469
+ "1.3.6.1.4.1.1000.1": {"1.3.6.1.2.1.31.1.1", "1.3.6.1.4.1.2000.1"},
1470
+ },
1471
+ },
1472
+ "skip index transformation": {
1473
+ profile: &ddsnmp.Profile{
1474
+ Definition: &ddprofiledefinition.ProfileDefinition{
1475
+ Metrics: []ddprofiledefinition.MetricsConfig{
1476
+ {
1477
+ MIB: "MY-MIB",
1478
+ Table: ddprofiledefinition.SymbolConfig{
1479
+ OID: "1.3.6.1.4.1.1000.1",
1480
+ Name: "myTable",
1481
+ },
1482
+ Symbols: []ddprofiledefinition.SymbolConfig{
1483
+ {OID: "1.3.6.1.4.1.1000.1.1.1", Name: "myMetric"},
1484
+ },
1485
+ MetricTags: []ddprofiledefinition.MetricTagConfig{
1486
+ {
1487
+ Symbol: ddprofiledefinition.SymbolConfigCompat{
1488
+ OID: "1.3.6.1.2.1.31.1.1.1.1",
1489
+ Name: "ifName",
1490
+ },
1491
+ Table: "ifXTable",
1492
+ Tag: "interface",
1493
+ IndexTransform: []ddprofiledefinition.MetricIndexTransform{
1494
+ {Start: 1, End: 5},
1495
+ },
1496
+ },
1497
+ },
1498
+ },
1499
+ {
1500
+ MIB: "IF-MIB",
1501
+ Table: ddprofiledefinition.SymbolConfig{
1502
+ OID: "1.3.6.1.2.1.31.1.1",
1503
+ Name: "ifXTable",
1504
+ },
1505
+ Symbols: []ddprofiledefinition.SymbolConfig{
1506
+ {OID: "1.3.6.1.2.1.31.1.1.1.18", Name: "ifAlias"},
1507
+ },
1508
+ },
1509
+ },
1510
+ },
1511
+ },
1512
+ expected: map[string][]string{}, // Should skip due to index transformation
1513
+ },
1514
+ "no cross-table tags": {
1515
+ profile: &ddsnmp.Profile{
1516
+ Definition: &ddprofiledefinition.ProfileDefinition{
1517
+ Metrics: []ddprofiledefinition.MetricsConfig{
1518
+ {
1519
+ MIB: "IF-MIB",
1520
+ Table: ddprofiledefinition.SymbolConfig{
1521
+ OID: "1.3.6.1.2.1.2.2",
1522
+ Name: "ifTable",
1523
+ },
1524
+ Symbols: []ddprofiledefinition.SymbolConfig{
1525
+ {OID: "1.3.6.1.2.1.2.2.1.10", Name: "ifInOctets"},
1526
+ },
1527
+ MetricTags: []ddprofiledefinition.MetricTagConfig{
1528
+ {
1529
+ Symbol: ddprofiledefinition.SymbolConfigCompat{
1530
+ OID: "1.3.6.1.2.1.2.2.1.2",
1531
+ Name: "ifDescr",
1532
+ },
1533
+ Tag: "interface",
1534
+ // No Table field means same table
1535
+ },
1536
+ },
1537
+ },
1538
+ },
1539
+ },
1540
+ },
1541
+ expected: map[string][]string{},
1542
+ },
1543
+ }
1544
+
1545
+ for name, tc := range tests {
1546
+ t.Run(name, func(t *testing.T) {
1547
+ ctrl := gomock.NewController(t)
1548
+ defer ctrl.Finish()
1549
+
1550
+ mockHandler := snmpmock.NewMockHandler(ctrl)
1551
+ collector := New(mockHandler, []*ddsnmp.Profile{tc.profile}, logger.New())
1552
+
1553
+ deps := collector.analyzeTableDependencies(tc.profile)
1554
+
1555
+ assert.Equal(t, len(tc.expected), len(deps), "Wrong number of tables with dependencies")
1556
+
1557
+ for tableOID, expectedDeps := range tc.expected {
1558
+ actualDeps, ok := deps[tableOID]
1559
+ assert.True(t, ok, "Missing dependencies for table %s", tableOID)
1560
+ assert.ElementsMatch(t, expectedDeps, actualDeps, "Wrong dependencies for table %s", tableOID)
1561
+ }
1562
+ })
1563
+ }
1564
+}
1565
+
1566
func mustCompileRegex(pattern string) *regexp.Regexp {
1567
re, err := regexp.Compile(pattern)
1568
if err != nil {