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

Adva SNMP Trap Handlers #10094

Merged
merged 39 commits into from Apr 29, 2019

Conversation

Projects
None yet
2 participants
@h-barnhart
Copy link
Contributor

commented Apr 11, 2019

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.

Adds some traps handles for Adva Ethernet devices. Includes unit tests for the handlers.

h-barnhart added some commits Nov 9, 2018

@murrant
Copy link
Member

left a comment

Looking great!

Just a few small things and it looks like you have some style fixes needed.

public function handle(Device $device, Trap $trap)
{
$raw = $trap->getRaw();
Log::event($raw, $device->device_id, 'trap', 2);

This comment has been minimized.

Copy link
@murrant

murrant Apr 12, 2019

Member

Dumping the entire trap into the eventlog in addition to the event below might be too much noise. (could this be debug code?)

This comment has been minimized.

Copy link
@h-barnhart

h-barnhart Apr 12, 2019

Author Contributor

I'm was wanting to submit this now under the idea of something is better than nothing, then circle back and improve it later. If you prefer I can remove it from this pull request and add it at a later time. I'd like to get the other traps included as soon as possible as coding out all cases for this trap might take some time.

This is an interface threshold trap that fires on a number user defined thresholds. For example it will send a trap if x number of 64 byte packets are received on an interface or if the interface is up but not passing any traffic. There are 48 thresholds that are monitored at 15 minute and 1 day intervals. So this particular handler has 96 possible cases. Additionally this is only for "access port" interfaces, there is a similar handler for "network port" interfaces. Hopefully I can do one and do a find and replace on the OID names, but it will take a bit to work.

How would you like to proceed?

This comment has been minimized.

Copy link
@h-barnhart

h-barnhart Apr 12, 2019

Author Contributor

Ok, I should probably have my coffee, read my code, then respond. Yep, there was debugging still in there. Going to fix that.

This comment has been minimized.

Copy link
@h-barnhart

h-barnhart Apr 12, 2019

Author Contributor

Fixed the debug and added the requested functionality.

Show resolved Hide resolved LibreNMS/Snmptrap/Handlers/AdvaAttributeChange.php

@murrant murrant added the SNMP Traps label Apr 12, 2019

$threshOid = $trap->getOidData($trap->findOid("CM-PERFORMANCE-MIB::cmEthernetAccPortThresholdVariable"));
Log::event("$ifName threshold exceeded for $interval. Threshold OID is $threshOid.", $device->device_id, 'trap', 2);
switch (true) {
case strpos($threshOid, 'CM-PERFORMANCE-MIB::cmEthernetAccPortStatsUAS') === 0:

This comment has been minimized.

Copy link
@murrant

murrant Apr 12, 2019

Member

FYI, we have a function called str_contains() it is a little nicer than strpos :)
Also, this would probably be nicer as an array of key/value pairs (oid and message) and a loop.

This comment has been minimized.

Copy link
@h-barnhart

h-barnhart Apr 15, 2019

Author Contributor

Made requested changes.

This comment has been minimized.

Copy link
@h-barnhart

h-barnhart Apr 16, 2019

Author Contributor

Not sure why the changes you made are causing errors. Functionally it all looks the same, just a few changes to where things are called.

This comment has been minimized.

Copy link
@h-barnhart

h-barnhart Apr 16, 2019

Author Contributor

I this is some kind of scope issue that I don't understand (new to php). If I call getThresholds from withing the handle() method and pass it to getThresholdMessage method it works. Would that be acceptable?

This comment has been minimized.

Copy link
@h-barnhart

h-barnhart Apr 16, 2019

Author Contributor

Found and fixed the issue.

h-barnhart and others added some commits Apr 15, 2019

}
return 'unknown';
}
public function getThresholds()

This comment has been minimized.

Copy link
@murrant

murrant Apr 24, 2019

Member

This seems to be duplicated

This comment has been minimized.

Copy link
@h-barnhart

h-barnhart Apr 24, 2019

Author Contributor

The function name is the same, but the array of values returned is not. One is for Ethernet access ports and the other is for Ethernet network ports. Really the only difference between the two arrays are three characters in the keys that identify the trap as coming from an access port (cmAccPortQosShaperStatsBT) or a network port (cmNetPortQosShaperStatsBT). I think I can refactor it down though.

@murrant murrant force-pushed the h-barnhart:snmphandler-adva-1 branch from ed796f4 to 1c55436 Apr 28, 2019

@murrant murrant merged commit 5115fa9 into librenms:master Apr 29, 2019

5 of 6 checks passed

codeclimate 27 issues 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

@h-barnhart h-barnhart deleted the h-barnhart:snmphandler-adva-1 branch Apr 29, 2019

@murrant murrant added the Device 🖥 label Apr 29, 2019

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

Adva SNMP Trap Handlers (librenms#10094)
* Added SNMP Trap Handlers for Adva Ethernet devices

* Fixed formatting.

* Fixed errors from previous commit.

* Refactored AdvaAttributeChange.php

* Updated a few handlers, added test script.

* Added changes to snmptraps.php to make handlers active

* Fixed issues found by travisci

* Missed two mistakes in travisci, fixed. Should be ready for human eyes.

* Added SNMP Trap Handlers for Adva Ethernet devices

* Fixed formatting.

* Fixed errors from previous commit.

* Refactored AdvaAttributeChange.php

* Updated a few handlers, added test script.

* Added changes to snmptraps.php to make handlers active

* Fixed issues found by travisci

* Missed two mistakes in travisci, fixed. Should be ready for human eyes.

* Added two tests.

* fixed error

* Updated handlers with changes introduced in 1.50

* Refactored and added a few tests

* Added test for StateChangeTraps

* Added AdvaObjectDeletionTest, still wip as I need to recapture flow del.

* Added Network Element Alarm Trap Test

* Added more tests, but they are wip.

* Finished traps handler tests, few handler refoactors.

* fixed style errors and refactored as requested

* made requested changes to threshold trap handlers

* removed a test script

* modified adva port threshold handler

* Update AdvaAccThresholdCrossingAlert.php

* Update AdvaNetThresholdCrossingAlert.php

* removed static method

* fixed mistake in AdvaNetThresholdCrossingAlert.php

spencerbutler added a commit to spencerbutler/librenms that referenced this pull request May 21, 2019

Adva SNMP Trap Handlers (librenms#10094)
* Added SNMP Trap Handlers for Adva Ethernet devices

* Fixed formatting.

* Fixed errors from previous commit.

* Refactored AdvaAttributeChange.php

* Updated a few handlers, added test script.

* Added changes to snmptraps.php to make handlers active

* Fixed issues found by travisci

* Missed two mistakes in travisci, fixed. Should be ready for human eyes.

* Added SNMP Trap Handlers for Adva Ethernet devices

* Fixed formatting.

* Fixed errors from previous commit.

* Refactored AdvaAttributeChange.php

* Updated a few handlers, added test script.

* Added changes to snmptraps.php to make handlers active

* Fixed issues found by travisci

* Missed two mistakes in travisci, fixed. Should be ready for human eyes.

* Added two tests.

* fixed error

* Updated handlers with changes introduced in 1.50

* Refactored and added a few tests

* Added test for StateChangeTraps

* Added AdvaObjectDeletionTest, still wip as I need to recapture flow del.

* Added Network Element Alarm Trap Test

* Added more tests, but they are wip.

* Finished traps handler tests, few handler refoactors.

* fixed style errors and refactored as requested

* made requested changes to threshold trap handlers

* removed a test script

* modified adva port threshold handler

* Update AdvaAccThresholdCrossingAlert.php

* Update AdvaNetThresholdCrossingAlert.php

* removed static method

* fixed mistake in AdvaNetThresholdCrossingAlert.php

@murrant murrant removed the Device 🖥 label May 23, 2019

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