Fixes snmp sender

[kamilernerd]

No description provided.

[KristianLyng]

I'd like to have the hardcoded OID/PDU thing resolved and preferably also the x.err.

Fixing reflection->regular type switching is nice-to-have, not required.

[KristianLyng]

I didn't realize this the last time I approved this, but this really doesn't require reflection.

You can write (copied from marshal.go as an example):

        switch value := v.(type) {
        case float64:
                d.Duration = time.Duration(value)
                return nil
        case string:
                var err error
                d.Duration, err = time.ParseDuration(value)
                if err != nil {
                        return err
                }
                return nil
        default:
                return errors.New("invalid duration")
        }

Reflection is considerably slower, which probably isn't noticeable in an SNMP trap sender, but should, in general, be avoided in busy loops.

[KristianLyng]

Are you sure you want to do this? This will DISABLE the entire sender on the first mal-formed or failing snmp trap. The pattern of using x.err is mainly meant to remember that initialization (x.init()) failed after sync.Once has run.

[KristianLyng]

The SNMPTrapOID thing needs to be properly resolved since it will dictate configuration we will have to support.

[KristianLyng]

This is still hard coded.

I did a little research. Turns out .1.3.6.1.6.3.1.1.4.1 is literally "snmpTrapOID" oid, and always part of a trap, however, the value needs to not be hard-coded, even if it can be overridden now. The value you provide is Telenor-specific, and the .2023 makes it very implementation-specific.

I suggest changing the configurable PduTypes to say "SnmpTrapOID" and being of type string, and then using that as value.

E.g.:

	pdutypes = []gosnmp.SnmpPDU{
			{
				Value: x.SnmpTrapOID,
				Name:  ".1.3.6.1.6.3.1.1.4.1.0",
				Type:  gosnmp.ObjectIdentifier,
			},

That does mean this wont work without configuration, but it shouldn't. The value here should always be configured, it's very much site-local and not something where we can assume a sensible global default exists.

See:

https://oidref.com/1.3.6.1.6.3.1.1.4.1
https://oidref.com/1.3.6.1.4.1.12748

A bonus would be to add to Verify() that it makes sure that the value is:

1. Actually provided.
2. Matches [0-9.]+