Skip to content

Commit

Permalink
[corechecks/snmp] Use getnext for reachable device status (#9189)
Browse files Browse the repository at this point in the history
  • Loading branch information
@AlexandreYang
AlexandreYang authored Sep 23, 2021
1 parent 94c93e4 commit 9b4bd54
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 32 deletions.
32 changes: 26 additions & 6 deletions pkg/collector/corechecks/snmp/devicecheck/devicecheck.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@ import (
"strings"
"time"

"github.com/cihub/seelog"

"github.com/DataDog/datadog-agent/pkg/metrics"
"github.com/DataDog/datadog-agent/pkg/util/log"

"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/checkconfig"
"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/common"
"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/fetch"
"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/gosnmplib"
"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/metadata"
"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/report"
"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/session"
Expand All @@ -21,6 +24,9 @@ import (
const (
snmpLoaderTag = "loader:core"
serviceCheckName = "snmp.can_check"

// 1.3 (iso.org) is the OID used for getNext call to check if the device is reachable
deviceReachableGetNextOid = "1.3"
)

// DeviceCheck hold info necessary to collect info for a single device
Expand Down Expand Up @@ -68,7 +74,7 @@ func (d *DeviceCheck) Run(collectionTime time.Time) error {
// Fetch and report metrics
var checkErr error
var deviceStatus metadata.DeviceStatus
tags, values, checkErr := d.getValuesAndTags(staticTags)
deviceReachable, tags, values, checkErr := d.getValuesAndTags(staticTags)
if checkErr != nil {
d.sender.ServiceCheck(serviceCheckName, metrics.ServiceCheckCritical, "", tags, checkErr.Error())
} else {
Expand All @@ -79,7 +85,7 @@ func (d *DeviceCheck) Run(collectionTime time.Time) error {
}

if d.config.CollectDeviceMetadata {
if values != nil {
if deviceReachable {
deviceStatus = metadata.DeviceStatusReachable
} else {
deviceStatus = metadata.DeviceStatusUnreachable
Expand All @@ -97,14 +103,15 @@ func (d *DeviceCheck) Run(collectionTime time.Time) error {
return checkErr
}

func (d *DeviceCheck) getValuesAndTags(staticTags []string) ([]string, *valuestore.ResultValueStore, error) {
func (d *DeviceCheck) getValuesAndTags(staticTags []string) (bool, []string, *valuestore.ResultValueStore, error) {
var deviceReachable bool
var checkErrors []string
tags := common.CopyStrings(staticTags)

// Create connection
connErr := d.session.Connect()
if connErr != nil {
return tags, nil, fmt.Errorf("snmp connection error: %s", connErr)
return false, tags, nil, fmt.Errorf("snmp connection error: %s", connErr)
}
defer func() {
err := d.session.Close()
Expand All @@ -113,7 +120,20 @@ func (d *DeviceCheck) getValuesAndTags(staticTags []string) ([]string, *valuesto
}
}()

err := d.doAutodetectProfile(d.session)
// Check if the device is reachable
getNextValue, err := d.session.GetNext([]string{deviceReachableGetNextOid})
if err != nil {
deviceReachable = false
checkErrors = append(checkErrors, fmt.Sprintf("check device reachable: failed: %s", err))
} else {
deviceReachable = true
if logLevel, err := log.GetLogLevel(); err != nil || logLevel == seelog.DebugLvl {
values := gosnmplib.ResultToScalarValues(getNextValue)
log.Debugf("check device reachable: success: %+v", values)
}
}

err = d.doAutodetectProfile(d.session)
if err != nil {
checkErrors = append(checkErrors, fmt.Sprintf("failed to autodetect profile: %s", err))
}
Expand All @@ -133,7 +153,7 @@ func (d *DeviceCheck) getValuesAndTags(staticTags []string) ([]string, *valuesto
if len(checkErrors) > 0 {
joinedError = errors.New(strings.Join(checkErrors, "; "))
}
return tags, valuesStore, joinedError
return deviceReachable, tags, valuesStore, joinedError
}

func (d *DeviceCheck) doAutodetectProfile(sess session.Session) error {
Expand Down
2 changes: 2 additions & 0 deletions pkg/collector/corechecks/snmp/devicecheck/devicecheck_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/checkconfig"
"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/common"
"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/gosnmplib"
"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/report"
"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/session"
)
Expand Down Expand Up @@ -144,6 +145,7 @@ profiles:
},
}

sess.On("GetNext", []string{"1.3"}).Return(&gosnmplib.MockValidReachableGetNextPacket, nil)
sess.On("Get", []string{"1.3.6.1.2.1.1.2.0"}).Return(&sysObjectIDPacket, nil)
sess.On("Get", []string{"1.3.6.1.2.1.1.3.0", "1.3.6.1.4.1.3375.2.1.1.2.1.44.0", "1.3.6.1.4.1.3375.2.1.1.2.1.44.999", "1.2.3.4.5", "1.3.6.1.2.1.1.5.0"}).Return(&packet, nil)
sess.On("GetBulk", []string{"1.3.6.1.2.1.2.2.1.13", "1.3.6.1.2.1.2.2.1.14", "1.3.6.1.2.1.31.1.1.1.1", "1.3.6.1.2.1.31.1.1.1.18"}, checkconfig.DefaultBulkMaxRepetitions).Return(&bulkPacket, nil)
Expand Down
14 changes: 14 additions & 0 deletions pkg/collector/corechecks/snmp/gosnmplib/testing_utils.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package gosnmplib

import "github.com/gosnmp/gosnmp"

// MockValidReachableGetNextPacket valid reachable packet
var MockValidReachableGetNextPacket = gosnmp.SnmpPacket{
Variables: []gosnmp.SnmpPDU{
{
Name: "1.3.6.1.2.1.1.2.0",
Type: gosnmp.ObjectIdentifier,
Value: "1.3.6.1.4.1.3375.2.1.3.4.1",
},
},
}
85 changes: 59 additions & 26 deletions pkg/collector/corechecks/snmp/snmp_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (

"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/checkconfig"
"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/common"
"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/gosnmplib"
"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/session"
)

Expand Down Expand Up @@ -168,6 +169,8 @@ tags:
},
}

sess.On("GetNext", []string{"1.3"}).Return(&gosnmplib.MockValidReachableGetNextPacket, nil)
sess.On("Get", mock.Anything).Return(&packet, nil)
sess.On("Get", mock.Anything).Return(&packet, nil)
sess.On("GetBulk", []string{"1.3.6.1.2.1.2.2.1.14", "1.3.6.1.2.1.2.2.1.2", "1.3.6.1.2.1.2.2.1.20"}, checkconfig.DefaultBulkMaxRepetitions).Return(&bulkPacket, nil)
sess.On("GetBulk", []string{"1.3.6.1.2.1.2.2.1.14.2", "1.3.6.1.2.1.2.2.1.2.2", "1.3.6.1.2.1.2.2.1.20.2"}, checkconfig.DefaultBulkMaxRepetitions).Return(&bulkPacket2, nil)
Expand Down Expand Up @@ -254,6 +257,7 @@ metrics:
},
}

sess.On("GetNext", []string{"1.3"}).Return(&gosnmplib.MockValidReachableGetNextPacket, nil)
sess.On("Get", mock.Anything).Return(&packet, nil)

err = chk.Run()
Expand Down Expand Up @@ -461,6 +465,7 @@ profiles:
},
}

sess.On("GetNext", []string{"1.3"}).Return(&gosnmplib.MockValidReachableGetNextPacket, nil)
sess.On("Get", []string{
"1.3.6.1.2.1.1.5.0",
"1.3.6.1.2.1.1.1.0",
Expand Down Expand Up @@ -717,6 +722,8 @@ func TestCheck_Run(t *testing.T) {
sessionConnError error
sysObjectIDPacket gosnmp.SnmpPacket
sysObjectIDError error
reachableGetNextError error
reachableValuesPacket gosnmp.SnmpPacket
valuesPacket gosnmp.SnmpPacket
valuesError error
expectedErr string
Expand All @@ -731,46 +738,62 @@ func TestCheck_Run(t *testing.T) {
name: "failed to fetch sysobjectid",
sysObjectIDError: fmt.Errorf("no sysobjectid"),
valuesPacket: valuesPacketUptime,
reachableValuesPacket: gosnmplib.MockValidReachableGetNextPacket,
expectedErr: "failed to autodetect profile: failed to fetch sysobjectid: cannot get sysobjectid: no sysobjectid",
expectedSubmittedMetrics: 1.0,
},
{
name: "unexpected values count",
expectedErr: "failed to autodetect profile: failed to fetch sysobjectid: expected 1 value, but got 0: variables=[]",
name: "unexpected values count",
reachableValuesPacket: gosnmplib.MockValidReachableGetNextPacket,
expectedErr: "failed to autodetect profile: failed to fetch sysobjectid: expected 1 value, but got 0: variables=[]",
},
{
name: "failed to fetch sysobjectid with invalid value",
reachableValuesPacket: gosnmplib.MockValidReachableGetNextPacket,
sysObjectIDPacket: sysObjectIDPacketInvalidValueMock,
expectedErr: "failed to autodetect profile: failed to fetch sysobjectid: error getting value from pdu: oid 1.3.6.1.2.1.1.2.0: ObjectIdentifier should be string type but got float64 type: gosnmp.SnmpPDU{Name:\"1.3.6.1.2.1.1.2.0\", Type:0x6, Value:1}",
},
{
name: "failed to fetch sysobjectid with invalid value",
sysObjectIDPacket: sysObjectIDPacketInvalidValueMock,
expectedErr: "failed to autodetect profile: failed to fetch sysobjectid: error getting value from pdu: oid 1.3.6.1.2.1.1.2.0: ObjectIdentifier should be string type but got float64 type: gosnmp.SnmpPDU{Name:\"1.3.6.1.2.1.1.2.0\", Type:0x6, Value:1}",
name: "failed to fetch sysobjectid with conversion error",
reachableValuesPacket: gosnmplib.MockValidReachableGetNextPacket,
sysObjectIDPacket: sysObjectIDPacketInvalidConversionMock,
expectedErr: "failed to autodetect profile: failed to fetch sysobjectid: error getting value from pdu: oid 1.3.6.1.2.1.1.2.0: ObjectIdentifier should be string type but got gosnmp.SnmpPDU type: gosnmp.SnmpPDU{Name:\"1.3.6.1.2.1.1.2.0\", Type:0x6, Value:gosnmp.SnmpPDU{Name:\"\", Type:0x0, Value:interface {}(nil)}}",
},
{
name: "failed to fetch sysobjectid with conversion error",
sysObjectIDPacket: sysObjectIDPacketInvalidConversionMock,
expectedErr: "failed to autodetect profile: failed to fetch sysobjectid: error getting value from pdu: oid 1.3.6.1.2.1.1.2.0: ObjectIdentifier should be string type but got gosnmp.SnmpPDU type: gosnmp.SnmpPDU{Name:\"1.3.6.1.2.1.1.2.0\", Type:0x6, Value:gosnmp.SnmpPDU{Name:\"\", Type:0x0, Value:interface {}(nil)}}",
name: "failed to fetch sysobjectid with error oid",
reachableValuesPacket: gosnmplib.MockValidReachableGetNextPacket,
sysObjectIDPacket: sysObjectIDPacketInvalidOidMock,
expectedErr: "failed to autodetect profile: failed to fetch sysobjectid: expect `1.3.6.1.2.1.1.2.0` OID but got `1.3.6.1.6.3.15.1.1.1.0` OID with value `{counter 123}`",
},
{
name: "failed to fetch sysobjectid with error oid",
sysObjectIDPacket: sysObjectIDPacketInvalidOidMock,
expectedErr: "failed to autodetect profile: failed to fetch sysobjectid: expect `1.3.6.1.2.1.1.2.0` OID but got `1.3.6.1.6.3.15.1.1.1.0` OID with value `{counter 123}`",
name: "failed to get profile sys object id",
reachableValuesPacket: gosnmplib.MockValidReachableGetNextPacket,
sysObjectIDPacket: sysObjectIDPacketInvalidSysObjectIDMock,
expectedErr: "failed to autodetect profile: failed to get profile sys object id for `1.999999`: failed to get most specific profile for sysObjectID `1.999999`, for matched oids []: cannot get most specific oid from empty list of oids",
},
{
name: "failed to get profile sys object id",
sysObjectIDPacket: sysObjectIDPacketInvalidSysObjectIDMock,
expectedErr: "failed to autodetect profile: failed to get profile sys object id for `1.999999`: failed to get most specific profile for sysObjectID `1.999999`, for matched oids []: cannot get most specific oid from empty list of oids",
name: "failed to fetch values",
reachableValuesPacket: gosnmplib.MockValidReachableGetNextPacket,
sysObjectIDPacket: sysObjectIDPacketOkMock,
valuesPacket: valuesPacketErrMock,
valuesError: fmt.Errorf("no value"),
expectedErr: "failed to fetch values: failed to fetch scalar oids with batching: failed to fetch scalar oids: fetch scalar: error getting oids `[1.3.6.1.2.1.1.3.0 1.3.6.1.4.1.3375.2.1.1.2.1.44.0 1.3.6.1.4.1.3375.2.1.1.2.1.44.999 1.2.3.4.5 1.3.6.1.2.1.1.5.0]`: no value",
},
{
name: "failed to fetch values",
sysObjectIDPacket: sysObjectIDPacketOkMock,
valuesPacket: valuesPacketErrMock,
valuesError: fmt.Errorf("no value"),
expectedErr: "failed to fetch values: failed to fetch scalar oids with batching: failed to fetch scalar oids: fetch scalar: error getting oids `[1.3.6.1.2.1.1.3.0 1.3.6.1.4.1.3375.2.1.1.2.1.44.0 1.3.6.1.4.1.3375.2.1.1.2.1.44.999 1.2.3.4.5 1.3.6.1.2.1.1.5.0]`: no value",
name: "failed to fetch sysobjectid and failed to fetch values",
reachableValuesPacket: gosnmplib.MockValidReachableGetNextPacket,
sysObjectIDError: fmt.Errorf("no sysobjectid"),
valuesPacket: valuesPacketErrMock,
valuesError: fmt.Errorf("no value"),
expectedErr: "failed to autodetect profile: failed to fetch sysobjectid: cannot get sysobjectid: no sysobjectid; failed to fetch values: failed to fetch scalar oids with batching: failed to fetch scalar oids: fetch scalar: error getting oids `[1.3.6.1.2.1.1.3.0]`: no value",
},
{
name: "failed to fetch sysobjectid and failed to fetch values",
sysObjectIDError: fmt.Errorf("no sysobjectid"),
valuesPacket: valuesPacketErrMock,
valuesError: fmt.Errorf("no value"),
expectedErr: "failed to autodetect profile: failed to fetch sysobjectid: cannot get sysobjectid: no sysobjectid; failed to fetch values: failed to fetch scalar oids with batching: failed to fetch scalar oids: fetch scalar: error getting oids `[1.3.6.1.2.1.1.3.0]`: no value",
name: "failed reachability check",
sysObjectIDError: fmt.Errorf("no sysobjectid"),
reachableGetNextError: fmt.Errorf("no value for GextNext"),
valuesPacket: valuesPacketErrMock,
valuesError: fmt.Errorf("no value"),
expectedErr: "check device reachable: failed: no value for GextNext; failed to autodetect profile: failed to fetch sysobjectid: cannot get sysobjectid: no sysobjectid; failed to fetch values: failed to fetch scalar oids with batching: failed to fetch scalar oids: fetch scalar: error getting oids `[1.3.6.1.2.1.1.3.0]`: no value",
},
}
for _, tt := range tests {
Expand Down Expand Up @@ -800,6 +823,7 @@ community_string: public

mocksender.SetSender(sender, chk.ID())

sess.On("GetNext", []string{"1.3"}).Return(&tt.reachableValuesPacket, tt.reachableGetNextError)
sess.On("Get", []string{"1.3.6.1.2.1.1.2.0"}).Return(&tt.sysObjectIDPacket, tt.sysObjectIDError)
sess.On("Get", []string{"1.3.6.1.2.1.1.3.0", "1.3.6.1.4.1.3375.2.1.1.2.1.44.0", "1.3.6.1.4.1.3375.2.1.1.2.1.44.999", "1.2.3.4.5", "1.3.6.1.2.1.1.5.0"}).Return(&tt.valuesPacket, tt.valuesError)
sess.On("Get", []string{"1.3.6.1.2.1.1.3.0"}).Return(&tt.valuesPacket, tt.valuesError)
Expand Down Expand Up @@ -861,6 +885,7 @@ metrics:
packet := gosnmp.SnmpPacket{
Variables: []gosnmp.SnmpPDU{},
}
sess.On("GetNext", []string{"1.3"}).Return(&gosnmplib.MockValidReachableGetNextPacket, nil)
sess.On("Get", []string{"1.2.3", "1.3.6.1.2.1.1.3.0"}).Return(&packet, nil)
sender.SetupAcceptAll()

Expand Down Expand Up @@ -1032,6 +1057,7 @@ tags:
},
},
}
sess.On("GetNext", []string{"1.3"}).Return(&gosnmplib.MockValidReachableGetNextPacket, nil)
var sysObjectIDPacket *gosnmp.SnmpPacket
sess.On("Get", []string{"1.3.6.1.2.1.1.2.0"}).Return(sysObjectIDPacket, fmt.Errorf("no value"))

Expand Down Expand Up @@ -1155,6 +1181,7 @@ tags:
sender.On("Commit").Return()

var nilPacket *gosnmp.SnmpPacket
sess.On("GetNext", []string{"1.3"}).Return(nilPacket, fmt.Errorf("no value for GetNext"))
sess.On("Get", []string{"1.3.6.1.2.1.1.2.0"}).Return(nilPacket, fmt.Errorf("no value"))

sess.On("Get", []string{
Expand All @@ -1164,8 +1191,10 @@ tags:
"1.3.6.1.2.1.1.3.0",
}).Return(nilPacket, fmt.Errorf("device failure"))

expectedErrMsg := "check device reachable: failed: no value for GetNext; failed to autodetect profile: failed to fetch sysobjectid: cannot get sysobjectid: no value; failed to fetch values: failed to fetch scalar oids with batching: failed to fetch scalar oids: fetch scalar: error getting oids `[1.3.6.1.2.1.1.5.0 1.3.6.1.2.1.1.1.0 1.3.6.1.2.1.1.2.0 1.3.6.1.2.1.1.3.0]`: device failure"

err = chk.Run()
assert.EqualError(t, err, "failed to autodetect profile: failed to fetch sysobjectid: cannot get sysobjectid: no value; failed to fetch values: failed to fetch scalar oids with batching: failed to fetch scalar oids: fetch scalar: error getting oids `[1.3.6.1.2.1.1.5.0 1.3.6.1.2.1.1.1.0 1.3.6.1.2.1.1.2.0 1.3.6.1.2.1.1.3.0]`: device failure")
assert.EqualError(t, err, expectedErrMsg)

snmpTags := []string{"snmp_device:1.2.3.5"}

Expand Down Expand Up @@ -1206,7 +1235,7 @@ tags:

sender.AssertEventPlatformEvent(t, compactEvent.String(), "network-devices-metadata")

sender.AssertServiceCheck(t, "snmp.can_check", metrics.ServiceCheckCritical, "", snmpTags, "failed to autodetect profile: failed to fetch sysobjectid: cannot get sysobjectid: no value; failed to fetch values: failed to fetch scalar oids with batching: failed to fetch scalar oids: fetch scalar: error getting oids `[1.3.6.1.2.1.1.5.0 1.3.6.1.2.1.1.1.0 1.3.6.1.2.1.1.2.0 1.3.6.1.2.1.1.3.0]`: device failure")
sender.AssertServiceCheck(t, "snmp.can_check", metrics.ServiceCheckCritical, "", snmpTags, expectedErrMsg)
}

func TestDiscovery(t *testing.T) {
Expand Down Expand Up @@ -1247,6 +1276,8 @@ metric_tags:
},
},
}

sess.On("GetNext", []string{"1.3"}).Return(&gosnmplib.MockValidReachableGetNextPacket, nil)
sess.On("Get", []string{"1.3.6.1.2.1.1.2.0"}).Return(&discoveryPacket, nil)

err := chk.Configure(rawInstanceConfig, []byte(``), "test")
Expand Down Expand Up @@ -1531,6 +1562,8 @@ metric_tags:
},
},
}

sess.On("GetNext", []string{"1.3"}).Return(&gosnmplib.MockValidReachableGetNextPacket, nil)
sess.On("Get", []string{"1.3.6.1.2.1.1.2.0"}).Return(&discoveryPacket, nil)

err = chk.Configure(rawInstanceConfig, []byte(``), "test")
Expand Down

0 comments on commit 9b4bd54

Please sign in to comment.