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

Add memory readings for Draytek OS #15618

Merged
merged 1 commit into from Dec 7, 2023

Conversation

Sweeny42
Copy link
Contributor

@Sweeny42 Sweeny42 commented Dec 4, 2023

Draytek use a custom OID to define their devices memory usage, defined here: https://www.draytek.com/support/knowledge-base/5517

This commit includes a mempool definition with the new memory OID 1.3.6.1.4.1.7367.3.7 and relevant LibreNMS tests

Please give a short description what your pull request is for

DO NOT DELETE THE UNDERLYING TEXT

Please note

Please read this information carefully. You can run ./lnms dev:check to check your code before submitting.

  • Have you followed our code guidelines?
  • If my Pull Request does some changes/fixes/enhancements in the WebUI, I have inserted a screenshot of it.
  • If my Pull Request makes discovery/polling/yaml changes, I have added/updated test data.

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.

@CLAassistant
Copy link

CLAassistant commented Dec 4, 2023

CLA assistant check
All committers have signed the CLA.

@PipoCanaja PipoCanaja added the Device 🖥️ New or added device support label Dec 5, 2023
@PipoCanaja
Copy link
Contributor

Hi @Sweeny42
Thanx for you PR. Tests need to be updated now. Could you try to do it ? if not, I'll help you.

@PipoCanaja PipoCanaja added the Needs Tests 🦄 https://docs.librenms.org/Developing/os/Test-Units/ label Dec 5, 2023
Copy link

github-actions bot commented Dec 5, 2023

Please add test data so we can ensure your change is not broken in the future.
Read the docs to find out how: https://docs.librenms.org/Developing/os/Test-Units

@Sweeny42 Sweeny42 force-pushed the draytek_memory branch 2 times, most recently from 17d756b to 4f8cfea Compare December 6, 2023 18:54
@Sweeny42
Copy link
Contributor Author

Sweeny42 commented Dec 6, 2023

Hi @PipoCanaja,
I've added the requested tests and rebased the changes, hopefully everything should now be there for your review. Please do let me know if there's anything further I need to commit.

@PipoCanaja
Copy link
Contributor

PipoCanaja commented Dec 6, 2023

Hi @PipoCanaja, I've added the requested tests and rebased the changes, hopefully everything should now be there for your review. Please do let me know if there's anything further I need to commit.

Hi @Sweeny42
Almost all good, only the JSON from draytek_vigor2962.snmprec is missing.

And for next time, you don't have to rebase everytime. Only if a change you do is conflicting with something in master. Rebasing and force-pushing can be risky sometimes.

Draytek use a custom OID to define their devices memory usage, defined here: https://www.draytek.com/support/knowledge-base/5517

This commit includes a mempool definition with the new memory OID `1.3.6.1.4.1.7367.3.7` and relevant LibreNMS tests
@Sweeny42
Copy link
Contributor Author

Sweeny42 commented Dec 7, 2023

Hi @PipoCanaja,
Ahh yes, my apologies! That should now be included.

Thank you for the info, I will remember for next time, it's a force of habit and as you say can be dangerous.

@PipoCanaja
Copy link
Contributor

Hi @Sweeny42
Looks good to me ! Thanx and congrats for your first PR in LibreNMS repo :)

@PipoCanaja PipoCanaja merged commit f6e7795 into librenms:master Dec 7, 2023
8 checks passed
@PipoCanaja PipoCanaja removed the Needs Tests 🦄 https://docs.librenms.org/Developing/os/Test-Units/ label Dec 7, 2023
@Sweeny42 Sweeny42 deleted the draytek_memory branch December 7, 2023 13:46
@librenms-bot
Copy link

This pull request has been mentioned on LibreNMS Community. There might be relevant details there:

https://community.librenms.org/t/24-1-0-changelog/23271/1

gunkaaa pushed a commit to gunkaaa/librenms that referenced this pull request Jan 8, 2024
Draytek use a custom OID to define their devices memory usage, defined here: https://www.draytek.com/support/knowledge-base/5517

This commit includes a mempool definition with the new memory OID `1.3.6.1.4.1.7367.3.7` and relevant LibreNMS tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Device 🖥️ New or added device support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants