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

Optional automatic sensor limits #9973

Merged
merged 5 commits into from Mar 16, 2019

Conversation

Projects
None yet
5 participants
@amigne
Copy link
Contributor

commented Mar 15, 2019

When a device does not give sensor limit values, LibreNMS automatically sets limits, based on the current value measured during the first discovery.

In environments where equipments providing such limit information and others that don't are mixed, it is impossible to determine at first if the limit value is provided by the vendor or by the tool.

In cases where the LibreNMS administrator would trust the vendor values (for instance, to generate alerts) and prefer have no values set at all when the vendor does not return any (these values can be manually set, if needed), a new configuration parameter can be set. If this parameter is true (default value), the discoverer will determine an automatic value; if the parameter is false, LibreNMS does not determine any automatic value when the device does not provide any.

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

amigne added some commits Mar 14, 2019

@CLAassistant

This comment has been minimized.

Copy link

commented Mar 15, 2019

CLA assistant check
All committers have signed the CLA.

@PipoCanaja PipoCanaja added the Feature label Mar 15, 2019

@murrant

This comment has been minimized.

Copy link
Member

commented Mar 16, 2019

The name of the config variable doesn't feel quite right. How about sensors.guess_limits?

@amigne

This comment has been minimized.

Copy link
Contributor Author

commented Mar 16, 2019

Thank you Tony for your feedback.

To be honest, naming the config variable was the less trivial part of the change.

I'll update the code with the proposed name and resubmit it.

Stay tuned...

@amigne amigne changed the title Optional automatic sensor limits WIP - Optional automatic sensor limits Mar 16, 2019

@TheGreatDoc

This comment has been minimized.

Copy link
Contributor

commented Mar 16, 2019

Do you mind also documenting this new option in https://docs.librenms.org/Support/Configuration/ ?

@amigne

This comment has been minimized.

Copy link
Contributor Author

commented Mar 16, 2019

Do you mind also documenting this new option in https://docs.librenms.org/Support/Configuration/ ?

@TheGreatDoc Thank you for the feedback. This is done.

@PipoCanaja

This comment has been minimized.

Copy link
Contributor

commented Mar 16, 2019

I guess you can remove the WIP tag in the title to get this merged.

@amigne amigne changed the title WIP - Optional automatic sensor limits Optional automatic sensor limits Mar 16, 2019

@PipoCanaja
Copy link
Contributor

left a comment

LGTM

@PipoCanaja PipoCanaja merged commit d15fda3 into librenms:master Mar 16, 2019

5 of 6 checks passed

codeclimate 1 issue to fix
Details
Inspection Summary
Details
Node: analysis
Details
Travis CI - Pull Request Build Passed
Details
WIP Ready for review
Details
license/cla Contributor License Agreement is signed.
Details

@amigne amigne deleted the amigne:optional-sensor-limits branch Mar 16, 2019

@PipoCanaja PipoCanaja referenced this pull request Mar 16, 2019

Merged

Do not use $sensor[sensor_limit] if not available #9978

1 of 1 task complete
@murrant

This comment has been minimized.

Copy link
Member

commented Mar 17, 2019

@amigne naming might be trivial, but it is the only thing we can't fix trivially ;)

funzoneq added a commit to funzoneq/librenms that referenced this pull request Apr 30, 2019

Optional automatic sensor limits (librenms#9973)
* Automatic sensor limits are now optional. Set ['discovery']['sensors']['autolimit'] = false to disable.
* Changing new config variable name from discovery.sensors.autolimit to sensors.guess_limits
* Updated documentation: ['sensors']['guess_limits'] value added

@lock lock bot locked as resolved and limited conversation to collaborators May 16, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.