Skip to content
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

Improve generator parse error handling #1167

Merged
merged 1 commit into from
May 5, 2024
Merged

Conversation

SuperQ
Copy link
Member

@SuperQ SuperQ commented May 1, 2024

Improve the handling of parse errors from net-snmp.

  • Fix the line split when there are no errors.
  • Default to failing on parse errors.
  • Add extra help messages to help when there are errors.
  • Improve parse_errors output.

@SuperQ
Copy link
Member Author

SuperQ commented May 1, 2024

So in previous combinations of the generator and net-snmp we would get missing MIB file information in the STDERR output of net-snmp. This would be easy to parse and print back to the user. But in the version I'm testing here the information is missing and I'm not sure why.

@SuperQ SuperQ requested a review from RichiH May 1, 2024 00:29
@SuperQ
Copy link
Member Author

SuperQ commented May 5, 2024

CC @bastischubert

@SuperQ SuperQ mentioned this pull request May 5, 2024
@bastischubert
Copy link
Contributor

bastischubert commented May 5, 2024

But in the version I'm testing here the information is missing and I'm not sure why.

Can you share which versions you're using ;)

@SuperQ
Copy link
Member Author

SuperQ commented May 5, 2024

It looks like the difference is between net-snmp 5.7 and 5.9. Previously we would show errors like this:

Cannot find module (SNMPv2-TC): At line 8 in mibs/HOST-RESOURCES-MIB

But that is not included anymore. If I use snmpwalk -m, it produces the error. So there is something about the way we call the C code that makes the errors not show up in newer net-snmp versions.

@SuperQ
Copy link
Member Author

SuperQ commented May 5, 2024

For now, I think we should go ahead with this change. It's still useful as-is. We can continue to work on the generator error parsing later.

@bastischubert
Copy link
Contributor

adding --snmp.mibopts=e produces the desired behaviour with net-snmp >= 5.9.x

@bastischubert
Copy link
Contributor

We should add that param to the Makefile

diff --git a/generator/Makefile b/generator/Makefile
index 579610d..bb7530a 100644
--- a/generator/Makefile
+++ b/generator/Makefile
@@ -70,7 +70,7 @@ generate: generator mibs
        MIBDIRS=$(MIB_PATH) ./generator --fail-on-parse-errors generate

 parse_errors: generator mibs
-       MIBDIRS=$(MIB_PATH) ./generator --fail-on-parse-errors parse_errors
+       MIBDIRS=$(MIB_PATH) ./generator --fail-on-parse-errors parse_errors --snmp.mibopts=e

 .PHONY: docker
 docker:```

@SuperQ SuperQ force-pushed the superq/better_mib_errors branch from b401d52 to a5bf664 Compare May 5, 2024 19:06
@SuperQ
Copy link
Member Author

SuperQ commented May 5, 2024

Easier, I added e to the default options.

@SuperQ SuperQ force-pushed the superq/better_mib_errors branch from a5bf664 to 2b05f72 Compare May 5, 2024 19:08
Improve the handling of parse errors from net-snmp.
* Fix the line split when there are no errors.
* Default to failing on parse errors.
* Add extra help messages to help when there are errors.
* Improve `parse_errors` output.

Signed-off-by: SuperQ <superq@gmail.com>
@SuperQ SuperQ force-pushed the superq/better_mib_errors branch from 2b05f72 to a99ae15 Compare May 5, 2024 19:09
@SuperQ
Copy link
Member Author

SuperQ commented May 5, 2024

The error output is now much easier to read:

$ MIBDIRS=mibs ./generator generate
ts=2024-05-05T19:09:48.112Z caller=net_snmp.go:175 level=info msg="Loading MIBs" from=mibs
ts=2024-05-05T19:09:48.258Z caller=main.go:177 level=warn msg="NetSNMP reported parse error(s)" errors=2453
ts=2024-05-05T19:09:48.258Z caller=main.go:183 level=error msg="Missing MIB" mib=AIRESPACE-REF-MIB from="At line 66 in mibs/AIRESPACE-WIRELESS-MIB"
ts=2024-05-05T19:09:48.337Z caller=main.go:135 level=error msg="Failing on reported parse error(s)" help="Use 'generator parse_errors' command to see errors, --no-fail-on-parse-errors to ignore"

@RichiH
Copy link
Member

RichiH commented May 5, 2024 via email

@SuperQ SuperQ merged commit bcbe2a5 into main May 5, 2024
7 checks passed
@SuperQ SuperQ deleted the superq/better_mib_errors branch May 5, 2024 20:02
@SuperQ SuperQ mentioned this pull request May 10, 2024
SuperQ pushed a commit that referenced this pull request May 11, 2024
* [CHANGE] Improve generator parse error handling #1167
* [ENHANCEMENT] generator: Add generator HELP override #1106
* [ENHANCEMENT] Refactoring of Scrape process, fixing multiple module issues #1111
* [ENHANCEMENT] Skip using an interactive terminal in "make docker-generate". #1113
* [ENHANCEMENT] Add SNMPInflight metric #1119
* [FEATURE] Support for passing username, password & priv_password as env vars #1074
* [FEATURE] Add GoSNMP logger #1157
* [FEATURE] Add a "snmp_context" parameter to the URL #1163
* [BUGFIX] generator: curl failed #1094
* [BUGFIX] Fix SNMPv3 password configuration #1122
* [BUGFIX] generator: Update generator User-Agent #1133
* [BUGFIX] generator: fix mibs directory specification for parse_errors command #1135
* [BUGFIX] generator: remove extra character from dell iDrac-SMIv1 MIB #1141
* [BUGFIX] Fix do not expand envvars for empty config fields #1148

snmp.yml changes:
* Updated Cisco MIBs #1180
* Updated Cyberpower MIBs #1124
* Updated servertech_sentry3 #1090
* Added support for Dell iDrac  #1125
---------

Signed-off-by: Sebastian Schubert <basti@schubert.digital>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants