-
Notifications
You must be signed in to change notification settings - Fork 31
[READY] Add SNMP scraping for network switches #1062
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
Conversation
Enable monitoring of Juniper switches via SNMP by adding switch targets to the prometheus configuration generator. This allows collecting switch metrics (port statistics, CPU, memory) alongside existing AP and Pi metrics. - Generate prom-switches.json targeting port 161 for SNMP polling - Configure Alloy with SNMP exporter for switch metrics collection
Change relative path from ../snmp.yml to ./snmp.yml since the file is in the same directory as monitoring.nix.
sarcasticadmin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@erikreinert thanks for working on all of this during the work party! Was this applied against core-master and able to scrape the 4200 we had running? I couldnt remember
I included some comments and did a little testing locally myself with the existing tests. I didnt want to bulldoze this branch but checkout https://github.com/socallinuxexpo/scale-network/tree/rh/snmp-scrape It encompasses more or less the items Im commenting on here.
As a follow up PR we should consider a related SNMP issue: #929 This would be so we dont have to embedded the snmp.yml in the repo
facts/inventory.py
Outdated
| if not vlanid.isdigit(): | ||
| return None | ||
| return { | ||
| "type": "vlan", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets remove this extra type is doesnt actually do anything
| with open(f"{outputdir}/prom-pis.json", "w") as f: | ||
| f.write(json.dumps(prom_pi_config, indent=2)) | ||
|
|
||
| prom_switch_config = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems reasonable, thanks for sticking with the existing convention
facts/test_inventory.py
Outdated
| assert inventory.populateservers(filename, mocvlans) == servers, filename | ||
|
|
||
|
|
||
| def test_generatepromconfigs(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i dont think these tests add a lot of value, seems to be mostly testing that open works. Lets remove these tests entirely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was my fault and has been fixed in #1060 and currently in master so we can drop this commit
| } | ||
|
|
||
| // Scrape SNMP metrics from switches | ||
| prometheus.scrape "switches" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
ce68d75 to
b0e3133
Compare
sarcasticadmin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated this PR with the subset of the original changes from rh/snmp-scrape
Summary
prom-switches.jsontargeting port 161 for SNMP polling of Juniper switches