Skip to content

Commit 06c7b3d

Browse files
authored
[receiver/snmpreceiver] fix scalar RA # check (#26363)
**Description:** Fixed a bug introduced by [#25174](#25174) There was check that was supposed to ensure that all resource attributes added to a metric were scalar, but it was really checking if the number of resource attributes on a metric was the same as the total number of possible scalar resource attributes defined in the config. This check has been updated to explicitly verify that all of the resource attributes on the metric are scalar. Bug examples: - A column metric indexed by a column attribute has two scalar resource attributes with different names but the same oid value. They are the only two scalar RAs defined in the config. Since the oids are added to a map whose length is checked, they are seen as one value and the comparison fails so many resources are created instead of one. - A column metric indexed by a column attribute has only one scalar resource attribute when two possible attributes are defined in the config. The check fails and many resources are created instead of one. **Testing:** - Test that a metric with one scalar resource attribute when two are available still creates only one resource - First bug does not feel necessary to unit test, it has been tested locally and is fixed. **Documentation:** - N/A
1 parent 169ac6c commit 06c7b3d

File tree

4 files changed

+161
-2
lines changed

4 files changed

+161
-2
lines changed
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: snmpreceiver
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Fix how to determine how many RAs on a metric are scalar
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [26363]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext: We now create the proper number of resources for configurations where a resource uses fewer than the available number of scalar resource attribtues.
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: [user]

receiver/snmpreceiver/scraper.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,9 +221,16 @@ func (s *snmpScraper) indexedDataToMetric(
221221
// with the row index of the SNMP data
222222
resourceAttributeNames := configHelper.getResourceAttributeNames(data.columnOID)
223223

224+
// Check how many of the resource attributes on this metric are scalar
225+
var numScalarResourceAttributes int
226+
for name := range resourceAttributes {
227+
if s.cfg.ResourceAttributes[name].ScalarOID != "" {
228+
numScalarResourceAttributes++
229+
}
230+
}
224231
var resourceKey string
225-
// If we only have scalar resource attributes, we don't need multiple resources
226-
if len(resourceAttributes) == len(columnOIDScalarResourceAttributeValues) {
232+
// If the only resource attributes on this metric are scalar, we don't need multiple resources
233+
if len(resourceAttributes) == numScalarResourceAttributes {
227234
resourceKey = getResourceKey(resourceAttributeNames, "")
228235
} else {
229236
resourceKey = getResourceKey(resourceAttributeNames, indexString)

receiver/snmpreceiver/scraper_test.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2982,6 +2982,103 @@ func TestScrape(t *testing.T) {
29822982
require.NoError(t, err)
29832983
},
29842984
},
2985+
{
2986+
desc: "Metric does not use all available scalar RAs and still behaves properly (near copy of (24)) (29)",
2987+
testFunc: func(t *testing.T) {
2988+
mockClient := new(MockClient)
2989+
scalarRA1 := SNMPData{
2990+
oid: ".5.0",
2991+
value: "scalar",
2992+
valueType: stringVal,
2993+
}
2994+
scalarRA2 := SNMPData{
2995+
oid: ".6.0",
2996+
value: "also scalar",
2997+
valueType: stringVal,
2998+
}
2999+
coidAttr11 := SNMPData{
3000+
columnOID: ".1",
3001+
oid: ".1.1",
3002+
value: "string1",
3003+
valueType: stringVal,
3004+
}
3005+
coidAttr12 := SNMPData{
3006+
columnOID: ".1",
3007+
oid: ".1.2",
3008+
value: "string2",
3009+
valueType: stringVal,
3010+
}
3011+
coid21 := SNMPData{
3012+
columnOID: ".2",
3013+
oid: ".2.1",
3014+
value: int64(3),
3015+
valueType: integerVal,
3016+
}
3017+
coid22 := SNMPData{
3018+
columnOID: ".2",
3019+
oid: ".2.2",
3020+
value: int64(4),
3021+
valueType: integerVal,
3022+
}
3023+
mockClient.On("Connect").Return(nil)
3024+
mockClient.On("Close").Return(nil)
3025+
mockClient.On("GetScalarData", []string{".5.0", ".6.0"}, mock.Anything).Return([]SNMPData{scalarRA1, scalarRA2}).Once()
3026+
mockClient.On("GetIndexedData", []string{".1"}, mock.Anything).Return([]SNMPData{coidAttr11, coidAttr12}).Once()
3027+
mockClient.On("GetIndexedData", []string{".2"}, mock.Anything).Return([]SNMPData{coid21, coid22}).Once()
3028+
scraper := &snmpScraper{
3029+
cfg: &Config{
3030+
ResourceAttributes: map[string]*ResourceAttributeConfig{
3031+
"rattr1": {
3032+
ScalarOID: ".5.0",
3033+
},
3034+
"rattr2": {
3035+
ScalarOID: ".6.0",
3036+
},
3037+
},
3038+
Attributes: map[string]*AttributeConfig{
3039+
"attr1": {
3040+
OID: ".1",
3041+
},
3042+
},
3043+
Metrics: map[string]*MetricConfig{
3044+
"metric1": {
3045+
Description: "test description",
3046+
Unit: "By",
3047+
Gauge: &GaugeMetric{
3048+
ValueType: "int",
3049+
},
3050+
ColumnOIDs: []ColumnOID{
3051+
{
3052+
OID: ".2",
3053+
ResourceAttributes: []string{"rattr1"},
3054+
Attributes: []Attribute{
3055+
{
3056+
Name: "attr1",
3057+
},
3058+
},
3059+
},
3060+
},
3061+
},
3062+
},
3063+
},
3064+
settings: receivertest.NewNopCreateSettings(),
3065+
client: mockClient,
3066+
logger: zap.NewNop(),
3067+
}
3068+
3069+
expectedMetricGen := func(t *testing.T) pmetric.Metrics {
3070+
goldenPath := filepath.Join("testdata", "expected_metrics", "29_metric_does_not_use_all_available_scalar_RAs_golden.yaml")
3071+
expectedMetrics, err := golden.ReadMetrics(goldenPath)
3072+
require.NoError(t, err)
3073+
return expectedMetrics
3074+
}
3075+
expectedMetrics := expectedMetricGen(t)
3076+
metrics, err := scraper.scrape(context.Background())
3077+
require.NoError(t, err)
3078+
err = pmetrictest.CompareMetrics(expectedMetrics, metrics, pmetrictest.IgnoreTimestamp())
3079+
require.NoError(t, err)
3080+
},
3081+
},
29853082
}
29863083

29873084
for _, tc := range testCases {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
resourceMetrics:
2+
- resource:
3+
attributes:
4+
- key: rattr1
5+
value:
6+
stringValue: scalar
7+
scopeMetrics:
8+
- metrics:
9+
- description: test description
10+
gauge:
11+
dataPoints:
12+
- asInt: "3"
13+
attributes:
14+
- key: attr1
15+
value:
16+
stringValue: string1
17+
timeUnixNano: "1651783494931319000"
18+
- asInt: "4"
19+
attributes:
20+
- key: attr1
21+
value:
22+
stringValue: string2
23+
timeUnixNano: "1651783494931319000"
24+
name: metric1
25+
unit: By
26+
scope:
27+
name: otelcol/snmpreceiver
28+
version: latest

0 commit comments

Comments
 (0)