-
Notifications
You must be signed in to change notification settings - Fork 645
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
"make generate" reports msg="NetSNMP reported parse error(s)" errors=2 #867
Comments
I noticed this too, but haven't had time to dig into it. Changing the generator config doesn't affect parse errors. Only changing the contents of the MIBs do. The parse errors happen before the config is used. |
Possibly to do with #508 ? |
I think it's partly to do with #508. I made this patch:
And now I get this output:
So the message "No log handling enabled - using stderr logging" is a spurious warning and can be ignored, but the one about DisplayString is real. Also, I can replicate that using snmptranslate:
|
OK, I think I found the problem:
All the other MIBs import DisplayString from The straightforward solution, then, is to fetch this file from https://raw.githubusercontent.com/net-snmp/net-snmp/v5.9/mibs/RFC1213-MIB.txt I can do a PR for that. It makes the generator problem go away (no error is returned). I see two other things which would be nice to do:
|
Hmm, there's a problem adding RFC1213-MIB; it overrides some of the things in IF-MIB. For example, comparing the original
and lots of comment differences too, e.g.
So the alternative is to patch those two MIB files to import DisplayString from SNMPv2-TC instead. What do you think about that??
"make generate" is also happy after that. (ISDN-MIB importing "transmission" from RFC1213-MIB doesn't cause any problem) With this change, the only difference between
(74 instances in total) |
Ahh, interesting. I guess we need to file some upstream issues with those MIBs? I'm not a big fan of patching MIBs, but, this seems easy enough. |
The trouble is, I don't think there's anything technically wrong with those MIBs, even if they're outdated. They're free to import DisplayString from an older MIB if they like. Arguably, the problem is that net-snmp cannot keep track of multiple definitions of the same OID depending on the context they are referred from; if multiple MIBs define the same OID, then one overwrites the other. If we could make the newer MIB take precedence, that would probably be OK. But I don't how to do that deterministically. Another option might be to provide a dummy version of the RFC1213-MIB which defines DisplayString but nothing else - that feels wrong though. Anyway, I've got a PR ready for patching the mibs at download time, you can see what you think. |
Fixes prometheus#867 Signed-off-by: Brian Candler <[email protected]>
Fixes prometheus#867 Signed-off-by: Brian Candler <[email protected]>
FWIW, SNMPv2-TC is derived from RFC 2579. Whilst that RFC doesn't specifically obsolete RFC 1213 or its descendants, it is of course more recent. Stuff like this is why I'm in favour of moving the MIB minefield out of this repo. Just run |
Yea, true. It's valid, but annoying. The generator.yml was never meant to be more than a simple example config to show off what's possible. Not be a universal config for all use cases. Some ideas:
I setup https://github.com/prometheus-community/snmp a while back to be a place to move the MIB minefield to. Something structured more like LibreNMS's MIB repo. Where we have various core MIBs available in dirs, and the vendor-specific stuff in their own dirs. Then at build time we select the collection of MIBDIRS for each generator.yml. Thinking about this more, I'm starting to like the idea of having MIBDIRS per module. This will make build slower since we need to iterate over the net-snmp code several times. But, it would allow for better handling of these conflicts. |
Yes - although I wonder if we'd also have to download each MIB into its own directory to prevent them stomping on each other. I'm not sure if specifying MIBS helps here, or whether net-snmp always reads in every file in every MIBDIRS, Alternatively in git we could build a custom MIBDIR for each module, containing symlinks to the modules to import. On top of this, I think it would be great if snmp_exporter itself accepted multiple snmp.yml files, e.g. it could read in |
Fixes prometheus#867 Signed-off-by: Brian Candler <[email protected]>
Cross-ref: #872 for long-term solution |
Fixes prometheus#867 Signed-off-by: Brian Candler <[email protected]>
…MIB (#868) Fixes #867 Signed-off-by: Brian Candler <[email protected]>
…MIB (prometheus#868) Fixes prometheus#867 Signed-off-by: Brian Candler <[email protected]> Signed-off-by: Stephan Windischmann <[email protected]>
Host operating system: output of
uname -a
(macOS 12.6.5, Intel)
snmp_exporter version: output of
snmp_exporter -version
I installed generator using
go install github.com/prometheus/snmp_exporter/generator@latest
on 2023-04-14generate
doesn't have a "version" or "--version" subcommand.What device/snmpwalk OID are you using?
N/A - using supplied generator.yml and
make generate
to fetch the MIBsIf this is a new device, please link to the MIB(s).
N/A
What did you do that produced an error?
What did you expect to see?
Completion without errors
What did you see instead?
"NetSNMP reported parse error(s)" errors=2
followed by make terminating. Full log:I note that snmptranslate seems happy when given a simple test:
It's unclear which MIBs have the parse errors in them. snmp.yml is created, but a bit smaller than the official one:
Diffing them, I see that some entries from CPS-MIB are different, but commenting out CyberPower from generator.yml doesn't make a difference.
The text was updated successfully, but these errors were encountered: