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 overrides for Synology MIB #556

Merged
merged 2 commits into from
Nov 28, 2020

Conversation

koraktor
Copy link
Contributor

This adds overrides for the Synology MIB to increase usefulness of the prodived metrics.

Human-readable strings are now provided as DisplayString values. The metrics raidFreeSize and raidTotalSize are now gauges.

@koraktor koraktor force-pushed the synology-overrides branch from 83c5572 to c9149e0 Compare August 23, 2020 09:00
@brian-brazil
Copy link
Contributor

Can you regenerate snmp.yml with the latest MIBs?

@koraktor koraktor force-pushed the synology-overrides branch from f806336 to deea5f6 Compare August 23, 2020 11:12
@brian-brazil
Copy link
Contributor

CI is still failing, make sure you have the latest mibs.

@koraktor
Copy link
Contributor Author

@brian-brazil Done.

But apparently there were a few unrelated changes that were caused by make clean mibs and now caused the generator tests to fail

@brian-brazil
Copy link
Contributor

Hmm, are you sure the generator is built from the latest head?

@koraktor
Copy link
Contributor Author

Yes, latest commit (before mine) is 1b3b053.

@brian-brazil
Copy link
Contributor

brian-brazil commented Aug 24, 2020

The only diff I'm seeing on head is for 300: cpri. By any chance are you on OSX? I've seen weirdness with netsnmp there.

@koraktor
Copy link
Contributor Author

Ok, that’s the problem. I can rebuild snmp.yml on a Linux system eventually, but it might take some time.

@SuperQ
Copy link
Member

SuperQ commented Aug 24, 2020

The cpri is new in the latest MIB (we don't version pin this MIB). #557

@SuperQ
Copy link
Member

SuperQ commented Aug 24, 2020

Ok, @koraktor, please rebase against head, that should fix up the unrelated diff.

@koraktor koraktor force-pushed the synology-overrides branch from deea5f6 to effa84d Compare August 24, 2020 12:53
@koraktor
Copy link
Contributor Author

Still failing because of the missing fixed_size: 6 lines. Probably the macOS problem?

@SuperQ
Copy link
Member

SuperQ commented Aug 24, 2020

That could be macos related. Depends on what version of net-snmp you've compiled against.

@koraktor
Copy link
Contributor Author

Building on Ubuntu results in the same file. Both Ubuntu and macOS use newer versions of Net-SNMP (5.8 and 5.9) than the Circle CI build (5.7).

@SuperQ
Copy link
Member

SuperQ commented Aug 25, 2020

I'm on Ubuntu 20.04/net-snmp 5.8. A freshly built generator doesn't drop the fixed_size: 6.

You could try using the Docker generator instructions, then it would match what's in CI.

@koraktor koraktor force-pushed the synology-overrides branch from effa84d to 37a6ea0 Compare August 27, 2020 07:47
@koraktor
Copy link
Contributor Author

koraktor commented Aug 27, 2020

I thought it wouldn’t be worth the hassle to fix the build for a text file that can be cleaned up manually. So I did that for the fixed_size: 6 lines.

Now there’s one last problem:
The override for the serialNumber is not applied in the CircleCI build.

@brian-brazil
Copy link
Contributor

Hmm, that's odd. It works for me locally too.

@koraktor
Copy link
Contributor Author

Rebased on master, still failing on CircleCI.

Signed-off-by: Sebastian Staudt <[email protected]>
Signed-off-by: Sebastian Staudt <[email protected]>
@koraktor
Copy link
Contributor Author

Rebased on master again.
Looks like the tests finally succeed now.

Any chance to get this merged now? @brian-brazil @SuperQ

@brian-brazil brian-brazil merged commit 6ae571d into prometheus:master Nov 28, 2020
@brian-brazil
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants