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

SNMP Table support addition #49

Merged
merged 2 commits into from
Mar 28, 2019
Merged

Conversation

axrayn
Copy link
Contributor

@axrayn axrayn commented Jan 14, 2019

@colinsurprenant Here's the PR for table support and bumps the version to 1.0.2.

This doesn't include the support for a config option to split table results rather than users relying on the split filter.

@colinsurprenant
Copy link
Contributor

thanks a lot @axrayn - looking at this!

@colinsurprenant
Copy link
Contributor

Overall looks great! only thing missing are some specs :P
I agree this plugin has a quite low coverage to start with, we should nonetheless at least add specs for the option parsing similar to what we have in the snmp_spec.rb file.
Otherwise, tests around the getTable operation can wait until we introduce a mock snmp server for these.
@axrayn let me know if you can/want to look into this or you'd like me to help with that.

@axrayn
Copy link
Contributor Author

axrayn commented Mar 27, 2019

Oops, my bad! I'll go through and see if I can add some specs and shout out if I get stuck.

@axrayn
Copy link
Contributor Author

axrayn commented Mar 27, 2019

@colinsurprenant Added specs for both the walk and tables functions (and then proceeded to git push before I'd commited the changes 🤦‍♂️ )

@colinsurprenant
Copy link
Contributor

@axrayn perfect! all code change LGTM! 🎉
A few more minor changes I'd like you to do:

  • I suggest we bump to 1.1.0 instead of 1.0.2, this is more than a bugfix!
  • Please add yourself in the CONTRIBUTORS file

Also, could you squash all code commits into one and the "version" related changes (logstash-input-snmp.gemspec, CHANGELOG, CONTRIBUTORS) into another commit with description "bump to version 1.1.0" ?

Let me know if you need help with git-fu for squashing etc.

Once the above is in I will merge and publish this!

Moved row insert out one layer to correct spot.

pin bundler version to < 2

[skip ci] Travis: update LOGSTASH_BRANCH from 6.[0..4] to 6.5

update matrix to include current targets [ci skip]

Update .travis.yml

Catching up to the master on logstash-input-snmp

Update snmp.rb

Updating error message about needing a get, walk, or table.

Update logstash-input-snmp.gemspec

Bumping plugin version

Update CHANGELOG.md

Updating changelog

Rebase

Adding spec tests for tables
@colinsurprenant
Copy link
Contributor

LGTM. Let's merge & publish! :shipit:
Great work @axrayn - thanks a lot for your contribution!

@colinsurprenant colinsurprenant merged commit 14b371d into logstash-plugins:master Mar 28, 2019
@colinsurprenant
Copy link
Contributor

v1.1.0 published and available for install.
I just realized we forgot to update docs/ will open a followup issue /cc @karenzone

@piellick
Copy link

piellick commented Mar 17, 2020

@colinsurprenant Here's the PR for table support and bumps the version to 1.0.2.

This doesn't include the support for a config option to split table results rather than users relying on the split filter.

Hi Everyone , great jobs table support work great !

Some one could provide a kv filter or split filter exemple to extract values of each SNMP index inside a document ?

exemple of table input :

{
  "1": 1,
  "2": "1.3.6.1.4.1.8708.2.30.2.3.1.1.2.1.76",
  "3": "1.3.6.1.4.1.8708.2.30.2.3.1.1.2.1.76",
  "4": "FDXXXXXXXXXXXXX",
  "5": 0,
  "6": 0,
  "7": 0,
  "8": 0,
  "9": "L2 service operation status is down",
  "10": 5,
  "11": "FDXXXXXXXXXXXXX",
  "12": "FDXXXXXXXXXXXXX",
  "13": 1,
  "14": "FDXXXXXXXXXXXXX",
  "15": "0.0.0.0",
  "index": "1"
},
{
  "1": 2,
  "2": "1.3.6.1.4.1.8708.2.5.2.5.1.0",
  "3": "1.3.6.1.4.1.8708.2.5.2.5.7.0",
  "4": "upload",
  "5": 0,
  "6": 0,
  "7": 1,
  "8": 4,
  "9": "Upload failed",
  "10": 4,
  "11": "FDXXXXXXXXXXXXX",
  "12": "0FDXXXXXXXXXXXXX",
  "13": 2,
  "14": "FDXXXXXXXXXXXXX",
  "15": "FDXXXXXXXXXXXXX",
  "index": "2"
}

i have posted a questionfor helping everyone like me who have trouble to extract value:
https://discuss.elastic.co/t/extract-snmp-table-from-snmp-input-plugin-with-key-value-or-split-filter/223991

i will propose a PR to update document just after. 👍

@karenzone
Copy link
Contributor

Thanks for advocating for quality docs, @colinsurprenant.
@piellick, thanks for opening the docs PR. When it's ready, please include me as a reviewer, too.

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.

4 participants