chore(go.d/ddsnmp): update vmBuildGroupKey in per-row mode without group_by (#21022)
Ilya Mashchenko committed
Sep 21, 2025 at 22:15 UTC
815f8f359d76fd1afd7d2af7246db425ddc38dd5
4 files changed
+118
-72
src/go/plugin/go.d/collector/snmp/collect_profiles.go
+25
-10
@@ -61,6 +61,9 @@ func (c *Collector) collectProfileTableMetrics(mx map[string]int64, pms []*ddsnm
61
}
62
63
key := tableMetricKey(m)
64
+ if key == "" {
65
+ continue
66
+ }
67
68
seen[key] = true
69
@@ -90,21 +93,33 @@ func (c *Collector) collectProfileTableMetrics(mx map[string]int64, pms []*ddsnm
93
}
94
95
func tableMetricKey(m ddsnmp.Metric) string {
93
- keys := make([]string, 0, len(m.Tags))
94
- for k := range m.Tags {
95
- keys = append(keys, k)
96
+ if m.Name == "" {
97
+ return ""
98
}
97
- sort.Strings(keys)
99
+
100
+ // Filter keys we actually use (skip "_" and empty values) and precompute final length.
101
+ include := make([]string, 0, len(m.Tags))
102
+ totalLen := len(m.Name)
103
+ for k, v := range m.Tags {
104
+ if v == "" || strings.HasPrefix(k, "_") {
105
+ continue
106
+ }
107
+ include = append(include, k)
108
+ totalLen += len("_") + len(v)
109
+ }
110
+ if len(include) == 0 {
111
+ return m.Name
112
+ }
113
+
114
+ sort.Strings(include)
115
116
var sb strings.Builder
117
+ sb.Grow(totalLen)
118
119
sb.WriteString(m.Name)
102
-
103
- for _, k := range keys {
104
- if v := m.Tags[k]; v != "" && !strings.HasPrefix(k, "_") {
105
- sb.WriteString("_")
106
- sb.WriteString(v)
107
- }
120
+ for _, k := range include {
121
+ sb.WriteByte('_')
122
+ sb.WriteString(m.Tags[k])
123
}
124
125
return sb.String()
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_vmetrics.go
+17
-17
@@ -310,28 +310,23 @@ func (p *vmetricsCollector) getDefinedMetricNames(profMetrics []ddprofiledefinit
310
311
// vmBuildGroupKey returns a stable group key.
312
func vmBuildGroupKey(tags map[string]string, agg *vmetricsAggregator) (string, bool) {
313
- if !agg.grouped {
313
+ if !agg.grouped || len(tags) == 0 {
314
return "", false
315
}
316
317
const (
318
- groupKeySep = '\x1F' // ASCII Unit Separator: safe delimiter between label values/pairs
319
- kvSep = '=' // used only in per-row fallback "k=v"
318
+ groupKeySep = '\x1F' // ASCII Unit Separator between values/pairs
319
+ kvSep = '=' // used in per-row fallback "k=v"
320
)
321
322
- if agg.perRow {
323
- if len(tags) == 0 {
324
- return "", false
325
- }
322
+ agg.keyBuf.Reset()
323
324
+ if agg.perRow {
325
if len(agg.groupBy) > 0 {
328
- agg.keyBuf.Reset()
326
for i, l := range agg.groupBy {
327
v := tags[l]
328
if v == "" {
332
- // missing hint
333
- agg.keyBuf.Reset()
334
- goto perRowFallback
329
+ return "", false
330
}
331
if i > 0 {
332
agg.keyBuf.WriteByte(groupKeySep)
@@ -341,14 +336,18 @@ func vmBuildGroupKey(tags map[string]string, agg *vmetricsAggregator) (string, b
336
return agg.keyBuf.String(), true
337
}
338
344
- perRowFallback:
345
- // Fallback: stable key from all tags (sorted k=v)
339
+ // per-row without group_by: stable key from all non-underscore tags
340
keys := make([]string, 0, len(tags))
341
for k := range tags {
348
- keys = append(keys, k)
342
+ if !strings.HasPrefix(k, "_") {
343
+ keys = append(keys, k)
344
+ }
345
+ }
346
+ if len(keys) == 0 {
347
+ return "", false
348
}
349
+
350
sort.Strings(keys)
351
- agg.keyBuf.Reset()
351
for i, k := range keys {
352
if i > 0 {
353
agg.keyBuf.WriteByte(groupKeySep)
@@ -360,14 +359,15 @@ func vmBuildGroupKey(tags map[string]string, agg *vmetricsAggregator) (string, b
359
return agg.keyBuf.String(), true
360
}
361
362
+ // non per-row: respect group_by exactly; underscore labels are NOT special
363
switch len(agg.groupBy) {
364
case 0:
365
return "", false
366
case 1:
367
- v := tags[agg.groupBy[0]]
367
+ l := agg.groupBy[0]
368
+ v := tags[l]
369
return v, v != ""
370
default:
370
- agg.keyBuf.Reset()
371
for i, l := range agg.groupBy {
372
v := tags[l]
373
if v == "" {
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_vmetrics_test.go
+75
-44
@@ -1062,50 +1062,6 @@ func TestVirtualMetricsCollector_Collect(t *testing.T) {
1062
},
1063
},
1064
1065
- "per_row hint missing for a row (fallback to full-tag key)": {
1066
- profileDef: &ddprofiledefinition.ProfileDefinition{
1067
- VirtualMetrics: []ddprofiledefinition.VirtualMetricConfig{
1068
- {
1069
- Name: "ifTrafficPerRowFallback",
1070
- PerRow: true,
1071
- GroupBy: []string{"interface"}, // hint missing on one row
1072
- Sources: []ddprofiledefinition.VirtualMetricSourceConfig{
1073
- {Metric: "ifHCInOctets", Table: "ifXTable", As: "in"},
1074
- {Metric: "ifHCOutOctets", Table: "ifXTable", As: "out"},
1075
- },
1076
- },
1077
- },
1078
- },
1079
- collectedMetrics: []ddsnmp.Metric{
1080
- // has 'interface'
1081
- {Name: "ifHCInOctets", Value: 10, IsTable: true, Table: "ifXTable",
1082
- Tags: map[string]string{"interface": "eth0", "ifIndex": "1"}},
1083
- {Name: "ifHCOutOctets", Value: 20, IsTable: true, Table: "ifXTable",
1084
- Tags: map[string]string{"interface": "eth0", "ifIndex": "1"}},
1085
- // missing 'interface' -> falls back to full-tag key
1086
- {Name: "ifHCInOctets", Value: 30, IsTable: true, Table: "ifXTable",
1087
- Tags: map[string]string{"name": "weird0", "ifIndex": "9"}},
1088
- {Name: "ifHCOutOctets", Value: 40, IsTable: true, Table: "ifXTable",
1089
- Tags: map[string]string{"name": "weird0", "ifIndex": "9"}},
1090
- },
1091
- expected: []ddsnmp.Metric{
1092
- {
1093
- Name: "ifTrafficPerRowFallback",
1094
- IsTable: true,
1095
- Table: "ifXTable",
1096
- Tags: map[string]string{"interface": "eth0", "ifIndex": "1"},
1097
- MultiValue: map[string]int64{"in": 10, "out": 20},
1098
- },
1099
- {
1100
- Name: "ifTrafficPerRowFallback",
1101
- IsTable: true,
1102
- Table: "ifXTable",
1103
- Tags: map[string]string{"name": "weird0", "ifIndex": "9"},
1104
- MultiValue: map[string]int64{"in": 30, "out": 40},
1105
- },
1106
- },
1107
- },
1108
-
1065
"per_row single-source (value path)": {
1066
profileDef: &ddprofiledefinition.ProfileDefinition{
1067
VirtualMetrics: []ddprofiledefinition.VirtualMetricConfig{
@@ -1569,6 +1525,81 @@ func TestVirtualMetricsCollector_Collect(t *testing.T) {
1525
}
1526
}
1527
1528
+func Test_vmBuildGroupKey(t *testing.T) {
1529
+ const sep = '\x1F'
1530
+
1531
+ tests := map[string]struct {
1532
+ agg vmetricsAggregator
1533
+ tags map[string]string
1534
+ wantOK bool
1535
+ wantKey string
1536
+ }{
1537
+ "not grouped -> no key": {
1538
+ agg: vmetricsAggregator{grouped: false},
1539
+ tags: map[string]string{"iface": "eth0"},
1540
+ wantOK: false,
1541
+ wantKey: "",
1542
+ },
1543
+
1544
+ "per_row + no groupBy: builds key from all non-underscore tags (k=v), sorted": {
1545
+ agg: vmetricsAggregator{grouped: true, perRow: true},
1546
+ tags: map[string]string{"iface": "eth0", "_if_type": "loopback", "zone": "a"},
1547
+ wantOK: true,
1548
+ wantKey: "iface=eth0" + string(sep) + "zone=a",
1549
+ },
1550
+
1551
+ "per_row + no groupBy: all tags underscore -> no key": {
1552
+ agg: vmetricsAggregator{grouped: true, perRow: true},
1553
+ tags: map[string]string{"_if_type": "loopback", "_role": "infra"},
1554
+ wantOK: false,
1555
+ wantKey: "",
1556
+ },
1557
+
1558
+ "per_row + groupBy: uses configured labels exactly; underscore NOT special": {
1559
+ agg: vmetricsAggregator{grouped: true, perRow: true, groupBy: []string{"_if_type", "iface"}},
1560
+ tags: map[string]string{"iface": "eth0", "_if_type": "ethernetCsmacd"},
1561
+ wantOK: true,
1562
+ wantKey: "ethernetCsmacd" + string(sep) + "eth0",
1563
+ },
1564
+
1565
+ "per_row + groupBy: missing required tag -> no key": {
1566
+ agg: vmetricsAggregator{grouped: true, perRow: true, groupBy: []string{"iface", "zone"}},
1567
+ tags: map[string]string{"iface": "eth0"},
1568
+ wantOK: false,
1569
+ wantKey: "",
1570
+ },
1571
+
1572
+ "non per_row + groupBy(1): returns that label value": {
1573
+ agg: vmetricsAggregator{grouped: true, perRow: false, groupBy: []string{"iface"}},
1574
+ tags: map[string]string{"iface": "eth1", "zone": "b"},
1575
+ wantOK: true,
1576
+ wantKey: "eth1",
1577
+ },
1578
+
1579
+ "non per_row + groupBy(2): underscore label NOT special and included": {
1580
+ agg: vmetricsAggregator{grouped: true, perRow: false, groupBy: []string{"_if_type", "zone"}},
1581
+ tags: map[string]string{"_if_type": "ethernetCsmacd", "zone": "edge"},
1582
+ wantOK: true,
1583
+ wantKey: "ethernetCsmacd" + string(sep) + "edge",
1584
+ },
1585
+
1586
+ "non per_row + groupBy: missing one value -> no key": {
1587
+ agg: vmetricsAggregator{grouped: true, perRow: false, groupBy: []string{"iface", "zone"}},
1588
+ tags: map[string]string{"iface": "eth2"},
1589
+ wantOK: false,
1590
+ wantKey: "",
1591
+ },
1592
+ }
1593
+
1594
+ for name, tc := range tests {
1595
+ t.Run(name, func(t *testing.T) {
1596
+ key, ok := vmBuildGroupKey(tc.tags, &tc.agg)
1597
+ assert.Equal(t, tc.wantOK, ok, "ok mismatch")
1598
+ assert.Equal(t, tc.wantKey, key, "key mismatch")
1599
+ })
1600
+ }
1601
+}
1602
+
1603
var (
1604
benchSinkString string
1605
benchSinkBucket *vmetricsGroupBucket
src/go/plugin/go.d/config/go.d/snmp.profiles/default/_std-if-mib.yaml
+1
-1
@@ -8,7 +8,7 @@ metrics:
8
name: ifNumber
9
chart_meta:
10
description: Number of network interfaces regardless of their current state present on this system
11
- family: 'Network/Interface/Count/Total'
11
+ family: 'Network/Interface/Count'
12
unit: "{interface}"
13
14
# =========================