diff --git a/pkg/collector/corechecks/snmp/internal/devicecheck/devicecheck.go b/pkg/collector/corechecks/snmp/internal/devicecheck/devicecheck.go index 0123f2c6423aec..0a07490d26746c 100644 --- a/pkg/collector/corechecks/snmp/internal/devicecheck/devicecheck.go +++ b/pkg/collector/corechecks/snmp/internal/devicecheck/devicecheck.go @@ -263,7 +263,7 @@ func (d *DeviceCheck) detectMetricsToMonitor(sess session.Session) error { } func (d *DeviceCheck) detectAvailableMetrics() ([]profiledefinition.MetricsConfig, []profiledefinition.MetricTagConfig) { - fetchedOIDs := session.FetchAllFirstRowOIDsUsingGetNext(d.session) + fetchedOIDs := session.FetchAllFirstRowOIDs(d.session) log.Debugf("fetched OIDs: %v", fetchedOIDs) root := common.BuildOidTrie(fetchedOIDs) diff --git a/pkg/collector/corechecks/snmp/internal/fetch/fetch.go b/pkg/collector/corechecks/snmp/internal/fetch/fetch.go index 8e0099a71232f5..e394b19f17b883 100644 --- a/pkg/collector/corechecks/snmp/internal/fetch/fetch.go +++ b/pkg/collector/corechecks/snmp/internal/fetch/fetch.go @@ -74,44 +74,46 @@ func getDeviceScanValues(sess session.Session, config *checkconfig.CheckConfig) if config.DeviceScanEnabled { // TODO: ONLY RUN once every X time - rootOid := config.DeviceScanLastOid // default root Oid - if rootOid == "" { - // NEW DEVICE SCAN - rootOid = defaultDeviceScanRootOid - config.DeviceScanCurScanStart = time.Now() - config.DeviceScanCurScanOidsCount = 0 - } + //rootOid := config.DeviceScanLastOid // default root Oid + //if rootOid == "" { + // // NEW DEVICE SCAN + // rootOid = defaultDeviceScanRootOid + // config.DeviceScanCurScanStart = time.Now() + // config.DeviceScanCurScanOidsCount = 0 + //} fetchStart := time.Now() - fetchedResults, lastOid, err := session.FetchAllOIDsUsingGetNext(sess, rootOid, config.DeviceScanMaxOidsPerRun) - if err != nil { - log.Warnf("[FetchAllOIDsUsingGetNext] error: %s", err) - return nil - } + fetchedResults := session.FetchAllFirstRowOIDsVariables(sess) + fetchDuration := time.Since(fetchStart) - log.Warnf("[FetchAllOIDsUsingGetNext] PRINT PDUs (len: %d)", len(results)) + log.Warnf("[FetchAllFirstRowOIDsVariables] PRINT PDUs (len: %d)", len(results)) for _, resultPdu := range fetchedResults { - log.Warnf("[FetchAllOIDsUsingGetNext] PDU: %+v", resultPdu) - } - config.DeviceScanCurScanOidsCount += len(fetchedResults) - - // TODO: ADD TELEMETRY for each check run - if len(fetchedResults) == config.DeviceScanMaxOidsPerRun { - log.Warnf("[FetchAllOIDsUsingGetNext] Partial Device Scan (Total Count: %d, Fetch Duration Ms: %d)", - config.DeviceScanCurScanOidsCount, - fetchDuration.Milliseconds(), - ) - // Partial Device Scan - config.DeviceScanLastOid = lastOid - } else { - log.Warnf("[FetchAllOIDsUsingGetNext] Full Device Scan (Total Count: %d, Duration: %.2f Sec)", - config.DeviceScanCurScanOidsCount, - time.Since(config.DeviceScanCurScanStart).Seconds(), - ) - // TODO: ADD TELEMETRY for complete device scan - // Full Device Scan completed - config.DeviceScanLastOid = "" + log.Warnf("[FetchAllFirstRowOIDsVariables] PDU: %+v", resultPdu) } + //config.DeviceScanCurScanOidsCount += len(fetchedResults) + + log.Warnf("[FetchAllFirstRowOIDsVariables] Device Scan (Total Count: %d, Duration: %.2f Sec)", + len(fetchedResults), + fetchDuration.Seconds(), + ) + + //// TODO: ADD TELEMETRY for each check run + //if len(fetchedResults) == config.DeviceScanMaxOidsPerRun { + // log.Warnf("[FetchAllOIDsUsingGetNext] Partial Device Scan (Total Count: %d, Fetch Duration Ms: %d)", + // config.DeviceScanCurScanOidsCount, + // fetchDuration.Milliseconds(), + // ) + // // Partial Device Scan + // //config.DeviceScanLastOid = lastOid + //} else { + // log.Warnf("[FetchAllOIDsUsingGetNext] Full Device Scan (Total Count: %d, Duration: %.2f Sec)", + // config.DeviceScanCurScanOidsCount, + // time.Since(config.DeviceScanCurScanStart).Seconds(), + // ) + // // TODO: ADD TELEMETRY for complete device scan + // // Full Device Scan completed + // //config.DeviceScanLastOid = "" + //} results = fetchedResults } return results diff --git a/pkg/collector/corechecks/snmp/internal/session/session.go b/pkg/collector/corechecks/snmp/internal/session/session.go index b4b090dc938dad..a2e46a7ea08396 100644 --- a/pkg/collector/corechecks/snmp/internal/session/session.go +++ b/pkg/collector/corechecks/snmp/internal/session/session.go @@ -175,10 +175,19 @@ func FetchSysObjectID(session Session) (string, error) { return strValue, err } -// FetchAllFirstRowOIDsUsingGetNext fetches all available OIDs +// FetchAllFirstRowOIDs fetches all available OIDs // Fetch all scalar OIDs and first row of table OIDs. -func FetchAllFirstRowOIDsUsingGetNext(session Session) []string { - var savedOIDs []string +func FetchAllFirstRowOIDs(session Session) []string { + var oids []string + variables := FetchAllFirstRowOIDsVariables(session) + for _, variable := range variables { + oids = append(oids, strings.TrimLeft(variable.Name, ".")) + } + return oids +} + +func FetchAllFirstRowOIDsVariables(session Session) []gosnmp.SnmpPDU { + var savedPDUs []gosnmp.SnmpPDU curRequestOid := "1.0" alreadySeenOIDs := make(map[string]bool) @@ -217,60 +226,60 @@ func FetchAllFirstRowOIDsUsingGetNext(session Session) []string { } alreadySeenOIDs[curRequestOid] = true - savedOIDs = append(savedOIDs, oid) + savedPDUs = append(savedPDUs, variable) } - return savedOIDs + return savedPDUs } -// FetchAllOIDsUsingGetNext fetches all available OIDs -func FetchAllOIDsUsingGetNext(session Session, rootOid string, maxOidsToFetch int) ([]gosnmp.SnmpPDU, string, error) { - var results []gosnmp.SnmpPDU - alreadySeenOIDs := make(map[string]bool) - - curOid := rootOid - // TODO: WHY gosnmp Walk does not work? - - //err := session.Walk(rootOid, func(dataUnit gosnmp.SnmpPDU) error { - // results = append(results, dataUnit) - // return nil - //}) - //if err != nil { - // log.Debugf("GetNext error: %s", err) - // return nil, err - //} - - oidsFetched := 0 - for { - curResults, err := session.GetNext([]string{curOid}) - if err != nil { - log.Debugf("GetNext error: %s", err) - break - } - - if len(curResults.Variables) != 1 { - log.Debugf("Expect 1 variable, but got %d: %+v", len(curResults.Variables), curResults.Variables) - break - } - variable := curResults.Variables[0] - if variable.Type == gosnmp.EndOfContents || variable.Type == gosnmp.EndOfMibView { - log.Debug("No more OIDs to fetch") - break - } - if alreadySeenOIDs[curOid] { - // breaking on already seen OIDs prevent infinite loop if the device mis behave by responding with non-sequential OIDs when called with GETNEXT - log.Debug("error: received non sequential OIDs") - break - } - alreadySeenOIDs[curOid] = true - - oid := strings.TrimLeft(variable.Name, ".") - curOid = oid - - results = append(results, variable) - oidsFetched += 1 - if oidsFetched >= maxOidsToFetch { - break - } - } - return results, curOid, nil -} +//// FetchAllOIDsUsingGetNext fetches all available OIDs +//func FetchAllOIDsUsingGetNext(session Session, rootOid string, maxOidsToFetch int) ([]gosnmp.SnmpPDU, string, error) { +// var results []gosnmp.SnmpPDU +// alreadySeenOIDs := make(map[string]bool) +// +// curOid := rootOid +// // TODO: WHY gosnmp Walk does not work? +// +// //err := session.Walk(rootOid, func(dataUnit gosnmp.SnmpPDU) error { +// // results = append(results, dataUnit) +// // return nil +// //}) +// //if err != nil { +// // log.Debugf("GetNext error: %s", err) +// // return nil, err +// //} +// +// oidsFetched := 0 +// for { +// curResults, err := session.GetNext([]string{curOid}) +// if err != nil { +// log.Debugf("GetNext error: %s", err) +// break +// } +// +// if len(curResults.Variables) != 1 { +// log.Debugf("Expect 1 variable, but got %d: %+v", len(curResults.Variables), curResults.Variables) +// break +// } +// variable := curResults.Variables[0] +// if variable.Type == gosnmp.EndOfContents || variable.Type == gosnmp.EndOfMibView { +// log.Debug("No more OIDs to fetch") +// break +// } +// if alreadySeenOIDs[curOid] { +// // breaking on already seen OIDs prevent infinite loop if the device mis behave by responding with non-sequential OIDs when called with GETNEXT +// log.Debug("error: received non sequential OIDs") +// break +// } +// alreadySeenOIDs[curOid] = true +// +// oid := strings.TrimLeft(variable.Name, ".") +// curOid = oid +// +// results = append(results, variable) +// oidsFetched += 1 +// if oidsFetched >= maxOidsToFetch { +// break +// } +// } +// return results, curOid, nil +//} diff --git a/pkg/collector/corechecks/snmp/internal/session/session_test.go b/pkg/collector/corechecks/snmp/internal/session/session_test.go index 015c594b15f8d2..ab275ee589082c 100644 --- a/pkg/collector/corechecks/snmp/internal/session/session_test.go +++ b/pkg/collector/corechecks/snmp/internal/session/session_test.go @@ -365,7 +365,7 @@ func TestFetchAllOIDsUsingGetNext(t *testing.T) { sess.On("GetNext", []string{"1.3.6.1.2.1.1.9.1.4"}).Return(CreateGetNextPacket("1.3.6.1.2.1.1.9.1.4.1", gosnmp.OctetString, []byte(`123`)), nil) sess.On("GetNext", []string{"1.3.6.1.2.1.1.9.1.5"}).Return(CreateGetNextPacket("999", gosnmp.EndOfMibView, nil), nil) - resultOIDs := FetchAllFirstRowOIDsUsingGetNext(sess) + resultOIDs := FetchAllFirstRowOIDs(sess) assert.Equal(t, []string{ "1.3.6.1.2.1.1.1.0", "1.3.6.1.2.1.1.2.0", @@ -385,7 +385,7 @@ func TestFetchAllOIDsUsingGetNext_invalidColumnOid(t *testing.T) { sess.On("GetNext", []string{"1.2.3.4.5"}).Return(CreateGetNextPacket("1.2.3.4.6.0", gosnmp.OctetString, []byte(`123`)), nil) sess.On("GetNext", []string{"1.2.3.4.6.0"}).Return(CreateGetNextPacket("999", gosnmp.EndOfMibView, nil), nil) - resultOIDs := FetchAllFirstRowOIDsUsingGetNext(sess) + resultOIDs := FetchAllFirstRowOIDs(sess) assert.Equal(t, []string{ "1.3.6.1.2.1.1.9.1.2.1", "1.2.3.4.5", @@ -402,7 +402,7 @@ func TestFetchAllOIDsUsingGetNext_handleNonSequentialOIDs(t *testing.T) { // invalid non-sequential oid that might lead to infinite loop if not handled sess.On("GetNext", []string{"1.2.3.4.5"}).Return(CreateGetNextPacket("1.2.3.4.5", gosnmp.OctetString, []byte(`123`)), nil) - resultOIDs := FetchAllFirstRowOIDsUsingGetNext(sess) + resultOIDs := FetchAllFirstRowOIDs(sess) assert.Equal(t, []string{ "1.3.6.1.2.1.1.9.1.2.1", "1.2.3.4.5", @@ -426,7 +426,7 @@ func TestFetchAllOIDsUsingGetNext_End(t *testing.T) { sess.On("GetNext", []string{"1.0"}).Return(CreateGetNextPacket("1.3.6.1.2.1.1.1.0", gosnmp.OctetString, []byte(`123`)), nil) sess.On("GetNext", []string{"1.3.6.1.2.1.1.1.0"}).Return(CreateGetNextPacket("1.3.6.1.2.1.1.2.0", gosnmp.OctetString, []byte(`123`)), nil) sess.On("GetNext", []string{"1.3.6.1.2.1.1.2.0"}).Return(CreateGetNextPacket("", test.valueType, nil), nil) - resultOIDs := FetchAllFirstRowOIDsUsingGetNext(sess) + resultOIDs := FetchAllFirstRowOIDs(sess) assert.Equal(t, []string{"1.3.6.1.2.1.1.1.0", "1.3.6.1.2.1.1.2.0"}, resultOIDs) }) @@ -440,7 +440,7 @@ func TestFetchAllOIDsUsingGetNext_invalidMoreThanOneVariables(t *testing.T) { packets.Variables = append(packets.Variables, packets.Variables[0]) sess.On("GetNext", []string{"1.0"}).Return(packets, nil) - resultOIDs := FetchAllFirstRowOIDsUsingGetNext(sess) // no packet created if variables != 1 + resultOIDs := FetchAllFirstRowOIDs(sess) // no packet created if variables != 1 assert.Equal(t, []string(nil), resultOIDs) } @@ -451,6 +451,6 @@ func TestFetchAllOIDsUsingGetNext_invalidZeroVariable(t *testing.T) { } sess.On("GetNext", []string{"1.0"}).Return(packets, nil) - resultOIDs := FetchAllFirstRowOIDsUsingGetNext(sess) // no packet created if variables != 1 + resultOIDs := FetchAllFirstRowOIDs(sess) // no packet created if variables != 1 assert.Equal(t, []string(nil), resultOIDs) } diff --git a/pkg/collector/corechecks/snmp/internal/session/utils.go b/pkg/collector/corechecks/snmp/internal/session/utils.go index 1a25166eb80e5e..c02be8eaea65ce 100644 --- a/pkg/collector/corechecks/snmp/internal/session/utils.go +++ b/pkg/collector/corechecks/snmp/internal/session/utils.go @@ -12,7 +12,7 @@ import ( ) // GetNextColumnOidNaive will return the next column OID for a given OID -// This is a naive algorithm based on detecting `.1.` (the table Entry oid that contains), increase the next digit and returns it. +// This is a naive algorithm based on detecting last `.1.` (expected to be table entry), increase the next digit and returns it. // Caveat: if the input OID index contain `.1.` the function won't return the next column OID but all the OID elements until `.1.`. func GetNextColumnOidNaive(oid string) (string, error) { oid = strings.TrimRight(strings.TrimLeft(oid, "."), ".")