Use gosmi for SNMP traps #9343
Use gosmi for SNMP traps #9343
Conversation
|
@loganmc10 Would mind reviewing this? |
| } | ||
| return out.Bytes(), nil | ||
| func (s *SnmpTrap) Init() error { | ||
| // must init, append path for each directory, laod module for every file |
There was a problem hiding this comment.
laod module for every file
typo here
|
|
||
| e.oidText = scanner.Text() | ||
| e.oidText = node.RenderQualified() | ||
| fmt.Printf("node %v\n", node) |
|
I think it looks good, I don't know about the unit tests, but I assume once they are passing that is good. I know that gosmi has a |
| require.Nil(t, s.Init()) | ||
| // Don't look up oid with snmptranslate. | ||
| s.execCmd = fakeExecCmd | ||
|
|
||
| require.NoError(t, s.Init()) | ||
|
|
There was a problem hiding this comment.
Oops, it looks like s.Init() was there originally. We must have removed it before we found it was definitely necessary and put it back :)
Co-authored-by: reimda <reimda@users.noreply.github.com>
Co-authored-by: reimda <reimda@users.noreply.github.com>
There was a problem hiding this comment.
Looks good in general, but I cannot test since I'm not using this plugin. I'm using the snmp plugin.
Anyway, I think it would be better to add the MIB path to the general snmp config (internal/snmp/config.go) and add the generic translation/lookup functions in that section, so both plugins can benefit from it.
go.mod
Outdated
| module github.com/influxdata/telegraf | ||
|
|
||
| go 1.16 | ||
| module github.com/golang-migrate/migrate/v4 |
There was a problem hiding this comment.
Is this intended? And relevant for this PR?
Looks like new artifacts were built from this PR. Get them here!Artifact URLs |
|
@MyaLongmire this PR will not solve #5720 in this current state. As it is about |
|
The readme still says it uses |
|
It is not correct and I will fix this in a new pr, good catch! Here is the pr for the fixed readme. |
Required for all PRs:
resolves #9307
config file now takes paths to where mib files are located
plugin now uses gosmi instead of snmptranslate
new unit test
added fake mibs for the tests