Implement indices filters

[sebastien-coavoux]

Hi,

This PR adds the feature of instance filtering in both the generator and the exporter.
Filters in the generator are referred as "static" while filters in the exporter are "dynamic".
The idea behind this, is to limit the number of oid returned by the monitored device , and in most case reduce the number of snmp get sent.

Static filters can be seen as an ease of configuration, as it make instance filtering easier to do
Dynamic filters are simple for now in order to keep the exporter stateless between runs. Filter are evaluated when /snmp route is reached by prometheus by http. We could have a /filter route to make 2 separate flows but that would complexify the code.

Filters are also not made to have multiple levels. Oid are targeted by a single filter, there is no "and / or" logic at all.

However, Dynamic filter evaluation support regex. Regex matching was as complex as string comparison but with more options for the user.

Let me know what do you think. I could squash commits / remove some if necessary

[SuperQ]

Nice, it will take some time to review this.

The first thing I am thinking about here is terminology. We use the term "instance" in Prometheus to refer to the target device.

I need to dig through various MIBs to see how the terminology is used there, there are some references to "INDEX", and some "instance".

From a high-level perspective, this is an "Index filter". It filters instances of results from the OID index(es). I think it would be more intuitive to align the naming in this way.

[sebastien-coavoux]

Hey, any update on it? I gpg signed the commit but now the generator fails because it is not finding some OID (raritan).

Pretty strange since I have touched the mib dir neither the generator yaml.

[SuperQ]

The generator thing happens when upstream MIBs cause issues.

#647 should fix that up.

[sebastien-coavoux]

Hi,

I've rebased the MR on main so that the CI is green. I've also changed the vocab from instance to index/indices ( I had to choose a side for the plural, feel free to argue on it 😄 )

Let me know if you would like something else.

[xkilian]

Been running with this for a while, over 800 switches/routers/firewalls with various filters. (Admin up, interface names and interface descriptions) No problems to report.

[SuperQ]

Sorry for the long delay in reviewing. Would you mind rebasing this?

[sebastien-coavoux]

Done !

[SuperQ]

This seems mostly sane, one question about the input config. Otherwise I think we can add this.

[sebastien-coavoux]

@SuperQ sorry for the delay, I was mostly off in summer. I've just added the index validation as part of Unmarshalling.

[PrestonTaylor]

This change is very well thought out, it worked flawlessly for my setup and accounted for all my whacky corner cases. It cut my time for stat collection down from 30s per router to 1.5!!!!! That's huge savings. Obviously it also cut my storage requirements greatly as well.

My only suggestion would be to make the example config in README.md a working one (filter targets should match oids listed in walk if I'm understanding correctly).

[sebastien-coavoux]

@PrestonTaylor , is there anything not clear in https://github.com/prometheus/snmp_exporter/pull/624/files#diff-a715bdcf88cd771d7ae09bd01863bab314ff194ba28e984689e9040d87846db9R147 ? Readme in generator provides an example, maybe you meant the Readme at the top level of the repo ?

[PrestonTaylor]

@sebastien-coavoux My suggestion was in your readme you've linked to add the oids you have in your target section (1.3.6.1.2.1.2.2.1.4 and bsnDot11EssSsid) to the walk section on line 61 of your change and 54 of the original. This was required in my case for it to output the metrics, without it it would query them according to the debug but not output them. Very minor change that most people would figure out anyway I'm sure. Git doesn't let me add a code suggestion more than 3 lines from your edits or I would do so.

[sebastien-coavoux]

@PrestonTaylor Ok I got it. I have added the 2 oid, the example provided is accurate with it. Thanks !

[sebastien-coavoux]

Updated PR from the input. Also rebased on main

[SuperQ]

Thanks @sebastien-coavoux, did you see #855?

[sebastien-coavoux]

I haven't dive into it but it looks like that the panic is not coming from the code modified by the PR.
Mar 20 18:56:07 myexporterserver snmp_exporter: panic: Unknown index type Bits comes from https://github.com/prometheus/snmp_exporter/blob/main/collector/collector.go#L747

The minimal case seems not to have filter to reproduce so I feel like it might be possible to reproduce it on main branch

I could have a look tonight, I guess the provided MIB in there would help reproduce.

[beorn7]

LGTM from the Go side.

[sebastien-coavoux]

Just minor Go style comments.

I have just looked at the shape of the Go code and didn't check correctness (assuming that was done by others).

Would adding tests be feasible?

Just noticed this, added some tests. Unless there is other comment I think I am done for that PR 👍

[JDA88]

From my understanding with this change it is now possible to use dynamic filters to only get interfaces that have ifAlias that match a RegEx but I struggle to find the correct syntax, does anyone have an example?

[sebastien-coavoux]

The readme should provide a good example of it. If not then maybe it is not clear enough so you could suggest a PR

From the snippet of readme we have

      dynamic: # dynamic filters are handed by the snmp exporter. The generator will simply pass on the configuration in the snmp.yml.
               # The exporter will do a snmp walk of the oid and will restrict snmp walk made on the targets
               # to the index matching the value in the values list.
               # This would be typically used to specify a filter for interfaces with a certain name in ifAlias, ifSpeed or admin status.
               # For example, only get interfaces that a gig and faster, or get interfaces that are named Up or interfaces that are admin Up
        - oid: 1.3.6.1.2.1.2.2.1.7
          targets:
            - "1.3.6.1.2.1.2.2.1.4"
          values: ["1", "2"]

This means that the exporter will walk 1.3.6.1.2.1.2.2.1.7 and for each index of this oid, if the values match "1" or "2" it will remember this index and do a snmp get of this index on the target .

example : 1.3.6.1.2.1.2.2.1.7.1 => 111 . The exporter will get 1.3.6.1.2.1.2.2.1.4.1

[JDA88]

I saw and understand the example but I fail to extrapolate It to my usercase.
Let say I want the:
ifAdminStatus (1.3.6.1.2.1.2.2.1.7) if the ifName (1.3.6.1.2.1.31.1.1.1.1) match the regex ^SW-.*
Will it translate to the following code?

      dynamic: 
        - oid: 1.3.6.1.2.1.31.1.1.1.1
          targets:
            - "1.3.6.1.2.1.2.2.1.7"
          values: ["^SW-.*"]

[sebastien-coavoux]

Yes this would be the right snippet of config.

If it is not working for you i would suggest to open an in issue. In the meantime things to check (if it is not working) :

• Run exporter in Debug, you should see in the logs the rules / filter evaluated
• Snmpwalk 1.3.6.1.2.1.31.1.1.1.1 manually and look at the output. Check if the regexp apply to that output : https://play.golang.com/p/hU-XCVNgEd5