Add handling of ping probe failed traps to juniper syslog trap handler

[johannaengland]

Scope and purpose

Dependent on #4080.

Reference: https://mibs.observium.org/mib/JUNIPER-SYSLOG-MIB/

This has been requested by the CNaaS engineering team. They want to get a notification when a ping probe fails, which happens when a juniper syslog trap with the event name PING_PROBE_FAILED is received. For now it is enough to open an event and the engineering team will close the event, because it is not clear yet what trap would have to be received to mark this event as closed again.

Contributor Checklist

Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to NAV can be found in the
Hacker's guide to NAV.

• Added a changelog fragment for towncrier
• Added/amended tests for new/changed code
• Added/changed documentation
• Linted/formatted the code with ruff, easiest by using pre-commit
• Wrote the commit message so that the first line continues the sentence "If applied, this commit will ...", starts with a capital letter, does not end with punctuation and is 50 characters or less long. See https://cbea.ms/git-commit/
• Based this pull request on the correct upstream branch: For a patch/bugfix affecting the latest stable version, it should be based on that version's branch (<major>.<minor>.x). For a new feature or other additions, it should be based on master.
• If applicable: Created new issues if this PR does not fix the issue completely/there is further work to be done
• If it's not obvious from a linked issue, described how to interact with NAV in order for a reviewer to observe the effects of this change first-hand (commands, URLs, UI interactions)
• If this results in changes in the UI: Added screenshots of the before and after
• If this adds a new Python source code file: Added the boilerplate header to that file

[github-actions]

Warning

15 of 15 new test names are missing convention keywords (given/when/then/it_should)

File	Test name
tests/integration/snmptrapd/sysloghandler_test.py	test_module_should_be_importable
tests/integration/snmptrapd/sysloghandler_test.py	test_event_and_alert_types_should_be_created_on_inializing
tests/integration/snmptrapd/sysloghandler_test.py	test_handler_should_log_trap
tests/integration/snmptrapd/sysloghandler_test.py	test_handler_should_ignore_traps_of_irrelevant_type
tests/integration/snmptrapd/sysloghandler_test.py	test_handler_should_ignore_traps_from_unknown_netbox
tests/integration/snmptrapd/sysloghandler_test.py	test_map_trap_attributes_matches_correct_attributes_and_values
tests/integration/snmptrapd/sysloghandler_test.py	test_handler_should_ignore_traps_of_irrelevant_event_name
tests/integration/snmptrapd/sysloghandler_test.py	test_handler_should_ignore_traps_indicating_state_change_to_other_than_ineligible_backup_or_active
tests/integration/snmptrapd/sysloghandler_test.py	test_handler_should_ignore_traps_indicating_state_change_to_backup_or_active_from_not_hold
tests/integration/snmptrapd/sysloghandler_test.py	test_handler_should_post_event_on_trap_indicating_ineligible_state
tests/integration/snmptrapd/sysloghandler_test.py	test_handler_should_post_event_on_trap_indicating_state_change_from_hold_to_backup
tests/integration/snmptrapd/sysloghandler_test.py	test_handler_should_post_event_on_trap_indicating_state_change_from_hold_to_active
tests/integration/snmptrapd/sysloghandler_test.py	test_handler_should_post_event_on_trap_indicating_state_change_from_ineligible_to_backup
tests/integration/snmptrapd/sysloghandler_test.py	test_handler_should_post_event_on_trap_indicating_state_change_from_ineligible_to_active
tests/integration/snmptrapd/sysloghandler_test.py	test_handler_should_post_event_on_trap_indicating_ping_probe_failed

Why am I seeing this?

Test names should follow a loose given/when/then pattern with keywords like given, when, then, or it_should. This is a suggestion, not a blocker. The check runs on new test methods added in this PR.

Examples:

• test_when_no_incidents_then_returns_empty_list
• test_it_should_create_incident_with_set_description
• test_given_expired_token_when_refreshing_then_raises_error

[github-actions]

Test results

    5 files      5 suites   8m 7s ⏱️
3 209 tests 3 209 ✅ 0 💤 0 ❌
6 342 runs  6 342 ✅ 0 💤 0 ❌

Results for commit f2d38ce.

♻️ This comment has been updated with latest results.

[codecov]

Codecov Report

❌ Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.64%. Comparing base (c450b37) to head (f2d38ce).

Files with missing lines	Patch %	Lines
python/nav/snmptrapd/handlers/sysloghandler.py	75.00%	3 Missing ⚠️

Additional details and impacted files

@@                           Coverage Diff                           @@
##           feature/snmptrap-juniper-sysloghandler    #4083   +/-   ##
=======================================================================
  Coverage                                   64.64%   64.64%           
=======================================================================
  Files                                         634      634           
  Lines                                       47098    47110   +12     
  Branches                                       43       43           
=======================================================================
+ Hits                                        30446    30455    +9     
- Misses                                      16642    16645    +3     
  Partials                                       10       10           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[johannaengland]

This is now in production at one customer, will not merge this until I've gotten confirmation that it works as intended

[lunkwill42]

This has been requested by the CNaaS engineering team.

That may very well be, but that still doesn't tell me much about what this is. It also lacks a separate news fragment that would have explained it.

Anyway, 14 test functions have been moved into a test class, but they all lack the initial self parameter that methods need: Most of them have db as their first argument, which now means that these tests do not actually invoke the db fixture at all, db becomes the self reference, since the instance always becomes the first argument of a method. This also means that these tests do not actually run inside a transaction and can potentially screw up the database for other tests.

I could also question the design:

This adds a new stateful event to NAV, but there is no state involved: A trap may open a state, but there is no corresponding trap to close the state...

Since there is no explanation for what problem this actually solves, I don't feel equipped to comment further on the design...

[johannaengland]

This is now in production at one customer, will not merge this until I've gotten confirmation that it works as intended

I've gotten the confirmation that it works as intented

[johannaengland]

This has been requested by the CNaaS engineering team.

That may very well be, but that still doesn't tell me much about what this is. It also lacks a separate news fragment that would have explained it.

This adds a new stateful event to NAV, but there is no state involved: A trap may open a state, but there is no corresponding trap to close the state...

I have now added a news fragment and updated the PR description a bit more. If more information is needed, we will need to ask the CNaaS engineers

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud