-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: snmp to use gosmi #9518
Merged
Merged
Changes from 73 commits
Commits
Show all changes
75 commits
Select commit
Hold shift + click to select a range
2541236
working on gosmi for fields
MyaLongmire 9e20fd0
added comments for next steps
MyaLongmire ae8fcb3
added mixed letters and numbers
MyaLongmire 2557a8c
more mixed oid support
MyaLongmire efd12fa
refactor: use gosmi instead of snmptranslate and snmptable
MyaLongmire d12fd8f
rework: table and fields init finction to point back to parent
MyaLongmire ac2ccaf
rework: got conversions working
MyaLongmire 5f577a7
added test mib files
MyaLongmire 2ec48c3
rework: unit tests
MyaLongmire a571d66
refactor: remove tablecache and lock
MyaLongmire d1097ee
fix: linter fixes
MyaLongmire e2f2c67
rework: update readme
MyaLongmire cb7f98a
Delete telegraf.conf
MyaLongmire e46f8f9
fix: spacing in sample config
f02f01d
fix: table test init test reworked
MyaLongmire aa6e5b2
fix: added more mibs for tests
MyaLongmire f90d915
fix: trying to get mib files working
MyaLongmire a6fc198
fix: removes unused code
MyaLongmire 4ad5668
fix: remove name from noTranslate test
MyaLongmire 345e591
fix: renaming
MyaLongmire bdd7029
fix: changed translate error to warning so telegraf can keep running,…
MyaLongmire 9ee6056
fix: call getMibsPath only once and start reqorking testTableBuild
MyaLongmire f0abade
fix: changed tableBuildWalk test to use a real mib added oids to inte…
MyaLongmire 233fe36
chore: removed leftover print statements
MyaLongmire ea369d3
chore: removed leftover print statements
MyaLongmire 8ee0c33
fis: trailing name in test
MyaLongmire ad6712e
fix: added .9 in front of oids in testSnmpInit_noTranslate so they do…
MyaLongmire 94fcfb1
Remove tests we added back during rebase
reimda 61a06fb
fix: removed isTag true from one field of test table init
MyaLongmire a4344cd
fix: split testTableBuild_walk into two tests. One has the original b…
MyaLongmire 861c212
fix: added the lock back in as gosmi is not thread safe
MyaLongmire 18684c8
chore: moved revision to snmp_v2 plugin and restored the original snm…
MyaLongmire 44263e1
fix: change warning back to err but do not return an error if the oid…
MyaLongmire 200e0e8
fix: move back to snmp
MyaLongmire 71fc43f
fix: remove error checking that never results in an error
MyaLongmire e7cd245
fix: remove extra comment
MyaLongmire 618cfe1
fix: made TestSnmpInit use a real table
MyaLongmire 74a37c0
fix: unit tests
MyaLongmire 090ca03
fix: server unit tests
MyaLongmire d94ebc3
fix: flipping line order to keep orginiality
MyaLongmire fb42465
fix: flipping line order to keep orginiality
MyaLongmire 5a1e57a
fix: fix spelling
MyaLongmire 7315bb7
fix: make the linter happy
MyaLongmire b8309e7
fix: linter issues
MyaLongmire 1423563
fix: added 9s back to testsnmpinit_notranslate
MyaLongmire f9b01f9
feat: update readme
MyaLongmire 46c7d9e
fix: remove error return type from getmibspath
MyaLongmire 7bac3e9
fix: readme linter errors
MyaLongmire 33132e0
fix: errors to warns
MyaLongmire 990521b
fix: use index/value pair directly
MyaLongmire c6663d7
fix: spelling
MyaLongmire a7b0b99
fix: use index/value pair directly
MyaLongmire 904ef43
fix: readme and linter
MyaLongmire 13677f3
fix: remove submask
MyaLongmire 611bad0
fix: added comments to getmibspath
MyaLongmire d356f14
chore: removed leftover comment
MyaLongmire 3e6f1b1
fix: changed filepath to error
2cf2fed
fix: filepath return an error
295d41b
chore: merge master to resolve conflicts
9445d02
fix: default path
054aafd
chore: readme markdown
MyaLongmire a77e8a3
Merge branch 'master' into snmp-gosmi-rework
MyaLongmire cddb46d
chore: marjdown linter fixes
MyaLongmire 5d241b5
move getmibspath to internal and append separate paths onto global pa…
MyaLongmire eec127c
refactor: move all gosmi code from snmp_trap into internal
MyaLongmire c7257bb
refactor: move gosmi into snmp internal and out of snmp
MyaLongmire 7701f3f
chore: added keys to composite literals
MyaLongmire d3d973e
chore: mute linter on too many return variables
MyaLongmire 45a424f
fix: initialise nil map
MyaLongmire d37ae61
chore: renaming and docs
MyaLongmire 194416e
refactor: clean up test
MyaLongmire 2167aac
chore: more doc clarity
MyaLongmire 92d3eef
chore: remove leftover comments
MyaLongmire 4beceeb
Merge branch 'master' into snmp-gosmi-rework
MyaLongmire 890ea6d
fix: added locking
MyaLongmire File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,183 @@ | ||
package snmp | ||
|
||
import ( | ||
"fmt" | ||
"os" | ||
"path/filepath" | ||
"strings" | ||
|
||
"github.com/influxdata/telegraf" | ||
"github.com/sleepinggenius2/gosmi" | ||
"github.com/sleepinggenius2/gosmi/types" | ||
) | ||
|
||
// must init, append path for each directory, load module for every file | ||
// or gosmi will fail without saying why | ||
func LoadMibsFromPath(paths []string, log telegraf.Logger) error { | ||
gosmi.Init() | ||
var folders []string | ||
for _, mibPath := range paths { | ||
gosmi.AppendPath(mibPath) | ||
folders = append(folders, mibPath) | ||
err := filepath.Walk(mibPath, func(path string, info os.FileInfo, err error) error { | ||
// symlinks are files so we need to double check if any of them are folders | ||
// Will check file vs directory later on | ||
if info.Mode()&os.ModeSymlink != 0 { | ||
link, err := os.Readlink(path) | ||
if err != nil { | ||
log.Warnf("Bad symbolic link %v", link) | ||
} | ||
folders = append(folders, link) | ||
} | ||
return nil | ||
}) | ||
if err != nil { | ||
return fmt.Errorf("Filepath could not be walked %v", err) | ||
} | ||
for _, folder := range folders { | ||
err := filepath.Walk(folder, func(path string, info os.FileInfo, err error) error { | ||
// checks if file or directory | ||
if info.IsDir() { | ||
gosmi.AppendPath(path) | ||
} else if info.Mode()&os.ModeSymlink == 0 { | ||
_, err := gosmi.LoadModule(info.Name()) | ||
if err != nil { | ||
log.Warnf("Module could not be loaded %v", err) | ||
} | ||
} | ||
return nil | ||
}) | ||
if err != nil { | ||
return fmt.Errorf("Filepath could not be walked %v", err) | ||
} | ||
} | ||
folders = []string{} | ||
} | ||
return nil | ||
} | ||
|
||
// The following is for snmp_trap | ||
type MibEntry struct { | ||
MibName string | ||
OidText string | ||
} | ||
|
||
func TrapLookup(oid string) (e MibEntry, err error) { | ||
var node gosmi.SmiNode | ||
node, err = gosmi.GetNodeByOID(types.OidMustFromString(oid)) | ||
|
||
// ensure modules are loaded or node will be empty (might not error) | ||
if err != nil { | ||
return e, err | ||
} | ||
|
||
e.OidText = node.RenderQualified() | ||
|
||
i := strings.Index(e.OidText, "::") | ||
if i == -1 { | ||
return e, fmt.Errorf("not found") | ||
} | ||
e.MibName = e.OidText[:i] | ||
e.OidText = e.OidText[i+2:] | ||
return e, nil | ||
} | ||
|
||
// The following is for snmp | ||
|
||
func GetIndex(oidNum string, mibPrefix string) (col []string, tagOids map[string]struct{}, err error) { | ||
// first attempt to get the table's tags | ||
tagOids = map[string]struct{}{} | ||
|
||
// mimcks grabbing INDEX {} that is returned from snmptranslate -Td MibName | ||
node, err := gosmi.GetNodeByOID(types.OidMustFromString(oidNum)) | ||
|
||
if err != nil { | ||
return []string{}, map[string]struct{}{}, fmt.Errorf("getting submask: %w", err) | ||
} | ||
|
||
for _, index := range node.GetIndex() { | ||
//nolint:staticcheck //assaignment to nil map to keep backwards compatibilty | ||
tagOids[mibPrefix+index.Name] = struct{}{} | ||
} | ||
|
||
// grabs all columns from the table | ||
// mimmicks grabbing everything returned from snmptable -Ch -Cl -c public 127.0.0.1 oidFullName | ||
col = node.GetRow().AsTable().ColumnOrder | ||
|
||
return col, tagOids, nil | ||
} | ||
|
||
//nolint:revive //Too many return variable but necessary | ||
func SnmpTranslateCall(oid string) (mibName string, oidNum string, oidText string, conversion string, err error) { | ||
MyaLongmire marked this conversation as resolved.
Show resolved
Hide resolved
|
||
var out gosmi.SmiNode | ||
var end string | ||
if strings.ContainsAny(oid, "::") { | ||
// split given oid | ||
// for example RFC1213-MIB::sysUpTime.0 | ||
s := strings.Split(oid, "::") | ||
// node becomes sysUpTime.0 | ||
node := s[1] | ||
if strings.ContainsAny(node, ".") { | ||
s = strings.Split(node, ".") | ||
// node becomes sysUpTime | ||
node = s[0] | ||
end = "." + s[1] | ||
} | ||
|
||
out, err = gosmi.GetNode(node) | ||
if err != nil { | ||
return oid, oid, oid, oid, err | ||
} | ||
|
||
oidNum = "." + out.RenderNumeric() + end | ||
} else if strings.ContainsAny(oid, "abcdefghijklnmopqrstuvwxyz") { | ||
//handle mixed oid ex. .iso.2.3 | ||
s := strings.Split(oid, ".") | ||
for i := range s { | ||
if strings.ContainsAny(s[i], "abcdefghijklmnopqrstuvwxyz") { | ||
out, err = gosmi.GetNode(s[i]) | ||
if err != nil { | ||
return oid, oid, oid, oid, err | ||
} | ||
s[i] = out.RenderNumeric() | ||
} | ||
} | ||
oidNum = strings.Join(s, ".") | ||
out, _ = gosmi.GetNodeByOID(types.OidMustFromString(oidNum)) | ||
} else { | ||
out, err = gosmi.GetNodeByOID(types.OidMustFromString(oid)) | ||
oidNum = oid | ||
// ensure modules are loaded or node will be empty (might not error) | ||
// do not return the err as the oid is numeric and telegraf can continue | ||
//nolint:nilerr | ||
if err != nil || out.Name == "iso" { | ||
return oid, oid, oid, oid, nil | ||
} | ||
} | ||
|
||
tc := out.GetSubtree() | ||
|
||
for i := range tc { | ||
// case where the mib doesn't have a conversion so Type struct will be nil | ||
// prevents seg fault | ||
if tc[i].Type == nil { | ||
break | ||
} | ||
switch tc[i].Type.Name { | ||
case "MacAddress", "PhysAddress": | ||
conversion = "hwaddr" | ||
case "InetAddressIPv4", "InetAddressIPv6", "InetAddress", "IPSIpAddress": | ||
conversion = "ipaddr" | ||
} | ||
} | ||
|
||
oidText = out.RenderQualified() | ||
i := strings.Index(oidText, "::") | ||
if i == -1 { | ||
return "", oid, oid, oid, fmt.Errorf("not found") | ||
} | ||
mibName = oidText[:i] | ||
oidText = oidText[i+2:] + end | ||
|
||
return mibName, oidNum, oidText, conversion, nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAICS, this function is called from each
inputs.snmp
plugin. Is this correct? If so, this means that you callgosmi.Init()
multiple time withgosmi
having a global state if my interpretation is correct...If this is the case, chances are high that you get side-effects between the different plugin-instances potentially ranging from race-conditions when adding the modules to mutual overwriting of the loaded plugins when
Init()
is called multiple times... To reduce those effects you should initgosmi
only once (usingsync.Once
) and guard this loading function with a lock. I don't know SNMP, but let's hope that there are no collisions possible between multiple (contradicting) mib-definitions...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do call
gosmi.Init
more than once but the library has a built-in check to see if it has already been initialized or not.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have also added a mutex
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @MyaLongmire, I don't see any check... When calling
gosmi.Init()
it calls this function which in turn callssmi.Init()
that can be found here. Already at this point, multiple files are reread etc. Additionallyinternal.Init()
is called also modifying data. While I agree that currently nothing "critical" is changed there it won't hurt to be on the safe side byand place
var once sync.Once
in the package scope, to be sure we call that function only once at least in that package. It would be even better to make sure it's not called elsewhere, but that's probably out of scope for this PR...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is added in pr 10199
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gosmi.Init() always calls smi.Init("gosmi") which always calls internal.Init(handleName) with the same handleName. That function does check whether it already has an internal handle of that name. That's why it looked to me like calling gosmi.Init() multiple times is safe.
It's a good idea to be defensive in our code and add sync.Once() anyway. I'm not sure gosmi intends it to be safe to call gosmi.Init() multiple times. I didn't see a guarantee in the docs, so even if it is now it might not be in a future version of gosmi.