feat(inputs.ipset): Add metric for number of entries and individual IPs #16124
Conversation
telegraf-tiger
bot
added
feat
verybadsoldier
force-pushed
the
feat/input/ipset_entries
branch
from
368af7d
to
28500db
Compare
…r of individual IPs
verybadsoldier
force-pushed
the
feat/input/ipset_entries
branch
from
28500db
to
c562e98
Compare
There was a problem hiding this comment.
Thanks @verybadsoldier for your contribution. I do have some comments in the code...
plugins/inputs/ipset/ipset.go
Outdated
| entryCounter := NewIpsetEntries(acc) | ||
|
|
There was a problem hiding this comment.
Do you really need to reinstantiate this in every gather cycle? How about doing it once in Init()?
| type IpsetEntries struct { | ||
| acc telegraf.Accumulator | ||
| initizalized bool | ||
| setName string | ||
| numEntries int | ||
| numIps int | ||
| } | ||
|
|
||
| func NewIpsetEntries(acc telegraf.Accumulator) *IpsetEntries { |
There was a problem hiding this comment.
Those two don't need to be exported as they are only used internally!
verybadsoldier
force-pushed
the
feat/input/ipset_entries
branch
from
3b048ee
to
60e0d89
Compare
|
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
|
Thanks for the review comments! |
|
Should I resolve the conversation I think I have solved or is that up to you? |
There was a problem hiding this comment.
Thanks for the update @verybadsoldier! A few more comments...
| lister: setList, | ||
| Timeout: defaultTimeout, | ||
| lister: setList, | ||
| entriesParser: ipsetEntries{}, |
There was a problem hiding this comment.
There is no need in doing this initialization as entriesParser should already be initialized like this.
| func (counter *ipsetEntries) addLine(line string, acc telegraf.Accumulator) { | ||
| data := strings.Fields(line) | ||
| if len(data) < 3 { | ||
| acc.AddError(fmt.Errorf("error parsing line (expected at least 3 fields): %s", line)) | ||
| return | ||
| } | ||
|
|
||
| operation := data[0] | ||
| if operation == "create" { | ||
| counter.commit(acc) | ||
| counter.initSet(data[1]) | ||
| } else if operation == "add" { | ||
| counter.entries++ | ||
|
|
||
| ip := data[2] | ||
| count, err := getCountInCidr(ip) | ||
| if err != nil { | ||
| acc.AddError(err) | ||
| return | ||
| } | ||
| counter.ips += count | ||
| } | ||
| } |
There was a problem hiding this comment.
Why don't you simply return an error and add that error to the acc in the Gather caller?
| operation := data[0] | ||
| if operation == "create" { | ||
| counter.commit(acc) | ||
| counter.initSet(data[1]) | ||
| } else if operation == "add" { | ||
| counter.entries++ | ||
|
|
||
| ip := data[2] | ||
| count, err := getCountInCidr(ip) | ||
| if err != nil { | ||
| acc.AddError(err) | ||
| return | ||
| } | ||
| counter.ips += count | ||
| } |
There was a problem hiding this comment.
| operation := data[0] | |
| if operation == "create" { | |
| counter.commit(acc) | |
| counter.initSet(data[1]) | |
| } else if operation == "add" { | |
| counter.entries++ | |
| ip := data[2] | |
| count, err := getCountInCidr(ip) | |
| if err != nil { | |
| acc.AddError(err) | |
| return | |
| } | |
| counter.ips += count | |
| } | |
| switch data[0] { | |
| case "create": | |
| counter.commit(acc) | |
| counter.initizalized = true | |
| counter.setName = data[1] | |
| counter.entries = 0 | |
| counter.ips = 0 | |
| case "add": | |
| counter.entries++ | |
| count, err := getCountInCidr(data[2]) | |
| if err != nil { | |
| return err | |
| } | |
| counter.ips += count | |
| } |
| fields := make(map[string]interface{}, 3) | ||
| fields["entries"] = counter.entries | ||
| fields["ips"] = counter.ips |
There was a problem hiding this comment.
Why three? I only see two fields? Anyway I think this is easier
| fields := make(map[string]interface{}, 3) | |
| fields["entries"] = counter.entries | |
| fields["ips"] = counter.ips | |
| fields := map[string]interface{}{ | |
| "entries": counter.entries, | |
| "ips": counter.ips, | |
| } |
| // reset counter and prepare for next usage | ||
| counter.reset() |
There was a problem hiding this comment.
| // reset counter and prepare for next usage | |
| counter.reset() | |
| // reset counter and prepare for next usage | |
| counter.initialized = false |
srebhan
changed the title
srebhan
changed the title
srebhan
changed the title
Summary
Some automatic blacklist systems (like e.g. FireHOL) are feeding known malicious IP addresses regularly into ipsets to be blocked by firewalls (e.g. iptables/nftables). It would be useful to be able to track the number of entries of such ipsets over time.
As entries can use CIDR notation to target a range of IPs (e.g. 10.12.4.0/8) with a single ipset entry, it is also useful to see the number of actualy IPs affected by one ipset.
Checklist
Related issues
resolves #16103