Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update Juniper module #1351

Merged
merged 5 commits into from
Feb 4, 2025
Merged

Update Juniper module #1351

merged 5 commits into from
Feb 4, 2025

Conversation

jacobw
Copy link
Contributor

@jacobw jacobw commented Jan 25, 2025

Update Juniper module to include FRU, DOM and basic subscriber count. Also use EnumAsStateSet to support alerting.

Signed-off-by: Jacob Winther <jacob@9.nz>
Signed-off-by: Jacob Winther <jacob@9.nz>
- JUNIPER-DOM-MIB::jnxDomCurrentTxLaserOutputPowerHighAlarmThreshold
- JUNIPER-DOM-MIB::jnxDomCurrentTxLaserOutputPowerLowAlarmThreshold
- JUNIPER-DOM-MIB::jnxDomCurrentTxLaserOutputPowerHighWarningThreshold
- JUNIPER-DOM-MIB::jnxDomCurrentTxLaserOutputPowerLowWarningThreshold
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if some of this should be a separate module, like juniper_optics.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used to break that out but found I keep on telling people they needed to add it, so I figure making it default makes it easier. It should only impact devices that have optics that support DOM.

Copy link
Member

Choose a reason for hiding this comment

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

I also used JUNIPER-DOM-MIB::jnxDomCurrentLaneTxLaserBiasCurrent to alert on "dying" optics when they start drawing more and more current.
We also used separate modules for basic juniper (switches,routers...) and more specific for devices with DOM support.

Copy link
Contributor Author

@jacobw jacobw Jan 27, 2025

Choose a reason for hiding this comment

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

I used to have jnxDomCurrentLaneTxLaserBiasCurrent but removed it because I didn't use it. I can see add that.

Regarding switches with DOM support, is there much benefit in removing it. Even small switches often have DOM for uplinks.

Happy to break DOM out though if that's the preference. My goal was to make life easier for users.

Copy link
Member

Choose a reason for hiding this comment

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

Since we added multi-module scrapes, I think splitting makes more sense. Users can request scrapes like if_mib,juniper,juniper_optics

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I'll make the changes tonight.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've broken optics out into it's own module, and included jnxDomCurrentLaneTxLaserBiasCurrent.

I also remove the threshold items as you can look at the jnxDomCurrentWarnings and jnxDomCurrentAlarms for threshold breaches, rather than calculating manually.

Signed-off-by: Jacob Winther <jacob@9.nz>
Signed-off-by: Jacob Winther <jacob@9.nz>
Signed-off-by: Jacob Winther <jacob@9.nz>
Copy link
Member

@bastischubert bastischubert left a comment

Choose a reason for hiding this comment

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

Fine for me

@bastischubert bastischubert requested a review from SuperQ January 28, 2025 21:08
Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Thanks

@SuperQ SuperQ merged commit a16bbb6 into prometheus:main Feb 4, 2025
6 checks passed
@SuperQ SuperQ mentioned this pull request Feb 6, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants