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

fix: snmp input plugin errors if mibs folder doesn't exist (#10346) #10354

Merged
merged 4 commits into from
Jan 11, 2022
Merged

fix: snmp input plugin errors if mibs folder doesn't exist (#10346) #10354

merged 4 commits into from
Jan 11, 2022

Conversation

sajoupa
Copy link
Contributor

@sajoupa sajoupa commented Dec 29, 2021

Required for all PRs:

resolves #10355

When there is no MIB folder, the snmp input plugin with exit with an error: e00147d.
This change makes it log a warning and continue (the plugin works fine even without MIBs).
It also adds a unit test which would have caught the issue.

@telegraf-tiger telegraf-tiger bot added the fix pr to fix corresponding bug label Dec 29, 2021
@powersj
Copy link
Contributor

powersj commented Jan 4, 2022

@MyaLongmire @reimda what is the behavior we want? Master currently stops Telegraf is a MIB is not found, this changes that to log the error, and keep processing.

@reimda
Copy link
Contributor

reimda commented Jan 4, 2022

@powersj We want it to be possible to use the snmp plugin without mibs, but also for telegraf to fail with an error if the user intentionally set the mib directory in telegraf.conf but made a typo or entered an invalid mib directory. It looks like this PR goes too far and makes all directory errors into warnings.

}
folders = append(folders, mibPath)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason this got moved?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in case we don't find any mibs, we don't want to append mibPath to folders.

internal/snmp/translate.go Show resolved Hide resolved
@MyaLongmire MyaLongmire added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Jan 11, 2022
@powersj
Copy link
Contributor

powersj commented Jan 11, 2022

Thank you!

@powersj powersj merged commit 457c98f into influxdata:master Jan 11, 2022
nward added a commit to nward/telegraf that referenced this pull request Jan 12, 2022
reimda pushed a commit that referenced this pull request Jan 27, 2022
@Hipska
Copy link
Contributor

Hipska commented Jan 31, 2022

@reimda @powersj @MyaLongmire Could it be that since this change, snmp_traps isn't loading anymore? #10548

@sajoupa sajoupa deleted the snmp-missing-mib-path branch February 2, 2022 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix pr to fix corresponding bug ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SNMP input plugin: error when the mib folder is missing
5 participants