[Discussion] Do we want to opportunistically parse Octetstring to work around broken vendor implementations?

[cukal]

Host operating system: output of uname -a

Linux xxxxx 3.10.0-693.17.1.el7.x86_64 #1 SMP Thu Jan 25 20:13:58 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

snmp_exporter version: output of snmp_exporter -version

snmp_exporter, version 0.8.0 (branch: HEAD, revision: 17cefcaf7010106861ce0faa2a9ec9e39b7c2b4b) build user: root@57f813d2791a build date: 20171120-12:36:57 go version: go1.9.2

What device/snmpwalk OID are you using?

Synology DS212j

If this is a new device, please link to the MIB(s).

https://global.download.synology.com/download/Document/MIBGuide/Synology_MIB_File.zip

What did you do that produced an error?

generator.yml:

modules:
  synology:
    walk:
      - 1.3.6.1.4.1.6574

The SYNOLOGY-SYSTEM-MIB contains:

modelName OBJECT-TYPE
    SYNTAX    OCTET STRING
    MAX-ACCESS  read-only
    STATUS    current
    DESCRIPTION
    "The Model name of this NAS"
    ::= { dsmInfo 1 }

Execute generator

./generator -generate

output:

INFO[0000] Loading MIBs from $HOME/.snmp/mibs:/usr/share/snmp/mibs  source="net_snmp.go:109"
WARN[0000] NetSNMP reported 1 parse errors               source="main.go:79"
INFO[0000] Generating config for module synology         source="main.go:36"
INFO[0000] Generated 139 metrics for module synology     source="main.go:39"
INFO[0000] Config written to /root/work/src/github.com/prometheus/snmp_exporter/generator/snmp.yml  source="main.go:63"

generated snmp.yml part for modelName:

  - name: modelName
    oid: 1.3.6.1.4.1.6574.1.5.1
    type: OctetString
    help: The Model name of this NAS - 1.3.6.1.4.1.6574.1.5.1

Created a snmp_exporter snmp.yml file and scraped the target.

What did you expect to see?

The same output as with a mib browser:

modelName.0 | DS212j | OctetString | 192.168.10.10:161

What did you see instead?

snmp_exporter returned the HEX encoded string.

# TYPE modelName gauge
modelName{modelName="0x44533231326A"} 1

[brian-brazil]

This behaviour is correct "OCTET STRING" is an arbitrary binary string, so there is no decoding we can do so we display it as hex. If this is in fact an ASCII or UTF-8 string you should request your vendor to change the type to DisplayString or SnmpAdminString respectively.

[RichiH]

You can manually edit the snmp.yml to account for this, but if there's other data in that OctetString you might get negative results.

@brian-brazil is there a sane way to mangle this is relabelling?

[brian-brazil]

No, non-UTF-8 label values will cause the scrape to fail before it gets to metric relabelling.

[RichiH]

I know.

The working assumption is that the data type is wrong, but only valid ASCII or UTF-8 will result. Given the context, I feel this assumption is valid. If it's invalid, I would argue that that is at some point outside of the scope of Prometheus ecosystem to handle as we have the fallback of using the original OctetString as a label value.

My question was in this context.

[cukal]

I spend some time manually editing the generated snmp.yml from generator, luckily enough the OctectString to DisplayString fixed the display problem. But when including another mib (1.3.6.1) to get other metrics snmp_exporter broke trying to split a received value into parts:

panic: runtime error: index out of range
goroutine 41 [running]:
main.indexOidsAsString(0xc421a2df00, 0x5, 0x8, 0xc42158f620, 0xb, 0x0, 0xc421678f70, 0x1, 0xc421678f08, 0x1, ...)
/go/src/github.com/prometheus/snmp_exporter/collector.go:358 +0x1e1d
main.indexesToLabels(0xc421d32c50, 0x6, 0x6, 0xc421f73300, 0xc4229f9b98, 0x0)
/go/src/github.com/prometheus/snmp_exporter/collector.go:411 +0x193
main.pduToSamples(0xc421d32c50, 0x6, 0x6, 0xc4229f9c00, 0xc421f73300, 0xc4229f9b98, 0xc421b87080, 0x1, 0x1)
/go/src/github.com/prometheus/snmp_exporter/collector.go:182 +0x77
main.collector.Collect(0xc422b44be1, 0xd, 0xc420120f00, 0xc422885260)
/go/src/github.com/prometheus/snmp_exporter/collector.go:157 +0x639
main.(*collector).Collect(0xc422b9e940, 0xc422885260)

I couldn't find out which value was choking it so I including only the oid's I really needed and got it working. After successfully walking it returned an UTF-8 error so I excluded those oid directly in the generated snmp.yml. The errors were all date related fields such as 1.3.6.1.2.1.25.1.2 hrSystemDate. After those edits the walk / scrape / relabelling works and I can use the metric output.

[brian-brazil]

That's why we can't treat octet strings as ASCII/UTF-8, but it shouldn't crash like that.

[RichiH]

Looking at what other tools in the SNMP space do: They either do this case by case (LibreNMS, Observium) or opportunistically (CLI tools, Zabbix) and https://godoc.org/golang.org/x/exp/utf8string#String.IsASCII exists.

That snmp_exporter is more brittle that it would need to be is an orthogonal discussion. I can see the point of slippery slopes, but I also maintain that snmp_exporter, unfortunately, exists in the real world with vendors which not ever fix their stuff.

[RichiH]

@cukal Can you open a new issue for your crash, please?

Re-opening and renaming this one for now.

[brian-brazil]

I can see the point of slippery slopes, but I also maintain that snmp_exporter, unfortunately, exists in the real world with vendors which not ever fix their stuff.

This is what #186 is for. It would be incorrect to do this automatically, as an Octect String could contain the ASCII value 0x1234. We only work from the MIBs.

[brian-brazil]

Dupe of #280

[llamafilm]

I'd like to add support for reconsidering this feature request. I'm new to Prometheus, and the first two SNMP devices I tried connecting both mis-label strings as OctetString, so I now have a huge generator.yml full of type overrides. I realize this could be fixed by the manufacturer's MIB, but this appears to be very common in the real world. Anyone familiar with Net-SNMP tools is accustomed to automatically decoding the hex as ASCII. I suggest adding a single key under each module in the generator like decode_hex: true. That would greatly simplify the setup and declutter the config file.

[RichiH]

It's at least worth another thought. CC @SuperQ