Merge pull request #193 from s-diez/fix-targets-loop

Fix loop over targets of all hosts

collector/collector.go

@@ -81,6 +81,10 @@ func NewRblCollector(rbls []string, targets []string, util *dns.DNSUtil, logger
 func (c *RblCollector) Describe(ch chan<- *prometheus.Desc) {
  ch <- c.configuredMetric
  ch <- c.blacklistedMetric
+ ch <- c.errorsMetrics
+ ch <- c.listedMetric
+ ch <- c.targetsMetric
+ ch <- c.durationMetric
 }
 
 // Collect ...
@@ -108,13 +112,18 @@ func (c *RblCollector) Collect(ch chan<- prometheus.Metric) {
  resolver := rbl.NewRBLResolver(c.logger, c.util)
 
  // iterate over hosts -> resolve to ip
- targets := make(chan []rbl.Target)
+ targets := make(chan rbl.Target)
+ wg := sync.WaitGroup{}
+ wg.Add(len(hosts))
+ go func() {
+ wg.Wait()
+ close(targets)
+ }()
  for _, host := range hosts {
- go resolver.Do(host, targets)
+ go resolver.Do(host, targets, wg.Done)
  }
-
  // run the check
- for _, target := range <-targets {
+ for target := range targets {
 
  results := make([]rbl.Result, 0)
 

collector/collector_test.go

@@ -0,0 +1,59 @@
+package collector_test
+
+import (
+ "strings"
+ "testing"
+
+ "github.com/Luzilla/dnsbl_exporter/collector"
+ "github.com/Luzilla/dnsbl_exporter/internal/tests"
+ "github.com/prometheus/client_golang/prometheus/testutil"
+ "github.com/stretchr/testify/assert"
+)
+
+func TestCollector(t *testing.T) {
+ dnsMock := tests.CreateDNSMock(t)
+ defer dnsMock.Close()
+
+ logger := tests.CreateTestLogger(t)
+ util := tests.CreateDNSUtil(t, dnsMock.LocalAddr())
+ rbls := []string{"zen.spamhaus.org", "cbl.abuseat.org"}
+ targets := []string{
+ "79.214.198.85", // bad
+ "relay.heise.de", // good
+ "1.3.3.7", // good
+ }
+
+ c := collector.NewRblCollector(rbls, targets, util, logger)
+
+ result, err := testutil.CollectAndLint(c)
+ assert.Empty(t, result)
+ assert.NoError(t, err)
+
+ // take all metrics but duration as it's value is hardly predictable
+ metrics := []string{}
+ for _, metric := range []string{"used", "ips_blacklisted", "errors", "listed", "targets"} {
+ metrics = append(metrics, collector.BuildFQName(metric))
+ }
+ expected := `
+ # HELP luzilla_rbls_ips_blacklisted Blacklisted IPs
+ # TYPE luzilla_rbls_ips_blacklisted gauge
+ luzilla_rbls_ips_blacklisted{hostname="1.3.3.7",ip="1.3.3.7",rbl="cbl.abuseat.org"} 0
+ luzilla_rbls_ips_blacklisted{hostname="1.3.3.7",ip="1.3.3.7",rbl="zen.spamhaus.org"} 0
+ luzilla_rbls_ips_blacklisted{hostname="79.214.198.85",ip="79.214.198.85",rbl="cbl.abuseat.org"} 0
+ luzilla_rbls_ips_blacklisted{hostname="79.214.198.85",ip="79.214.198.85",rbl="zen.spamhaus.org"} 1
+ luzilla_rbls_ips_blacklisted{hostname="relay.heise.de",ip="193.99.145.50",rbl="cbl.abuseat.org"} 0
+ luzilla_rbls_ips_blacklisted{hostname="relay.heise.de",ip="193.99.145.50",rbl="zen.spamhaus.org"} 0
+ # HELP luzilla_rbls_listed The number of listings in RBLs (this is bad)
+ # TYPE luzilla_rbls_listed gauge
+ luzilla_rbls_listed{rbl="cbl.abuseat.org"} 0
+ luzilla_rbls_listed{rbl="zen.spamhaus.org"} 1
+ # HELP luzilla_rbls_targets The number of targets that are being probed (configured via targets.ini or ?target=)
+ # TYPE luzilla_rbls_targets gauge
+ luzilla_rbls_targets 3
+ # HELP luzilla_rbls_used The number of RBLs to check IPs against (configured via rbls.ini)
+ # TYPE luzilla_rbls_used gauge
+ luzilla_rbls_used 2
+ `
+ err = testutil.CollectAndCompare(c, strings.NewReader(expected), metrics...)
+ assert.NoError(t, err)
+}

pkg/rbl/rbl_test.go

@@ -2,6 +2,7 @@ package rbl_test
 
 import (
  "net"
+ "sync"
  "testing"
 
  "github.com/Luzilla/dnsbl_exporter/internal/tests"
@@ -20,18 +21,22 @@ func TestRblSuite(t *testing.T) {
 
  resolver := rbl.NewRBLResolver(logger, d)
 
- targets := make(chan []rbl.Target)
- defer close(targets)
- go resolver.Do("relay.heise.de", targets)
+ targets := make(chan rbl.Target)
+ wg := sync.WaitGroup{}
+ wg.Add(1)
+ go func() {
+ wg.Wait()
+ close(targets)
+ }()
+ go resolver.Do("relay.heise.de", targets, wg.Done)
 
- for _, ip := range <-targets {
+ for ip := range targets {
  for _, blocklist := range []string{"cbl.abuseat.org", "zen.spamhaus.org"} {
  c := make(chan rbl.Result)
- defer close(c)
-
  r.Update(ip, blocklist, c)
-
  res := <-c
+ close(c)
+
  assert.False(t, res.Error)
  assert.NoError(t, res.ErrorType)
  }
@@ -117,10 +122,16 @@ func TestResolver(t *testing.T) {
 
  resolver := rbl.NewRBLResolver(logger, tests.CreateDNSUtil(t, dnsMock.LocalAddr()))
 
- c := make(chan []rbl.Target)
- go resolver.Do("relay.heise.de", c)
+ c := make(chan rbl.Target)
+ wg := sync.WaitGroup{}
+ wg.Add(1)
+ go func() {
+ wg.Wait()
+ close(c)
+ }()
+ go resolver.Do("relay.heise.de", c, wg.Done)
 
- for _, ip := range <-c {
+ for ip := range c {
  assert.Equal(t, "relay.heise.de", ip.Host)
  assert.NotEmpty(t, ip.IP.String())
  }

pkg/rbl/target.go

@@ -25,18 +25,16 @@ func NewRBLResolver(logger *slog.Logger, util *dns.DNSUtil) *Resolver {
  }
 }
 
-func (r *Resolver) Do(target string, c chan<- []Target) {
- var ips []Target
+func (r *Resolver) Do(target string, c chan<- Target, done func()) {
+ defer done()
 
  addr := net.ParseIP(target)
  if addr != nil {
  // already an IP
  r.logger.Info("we had an ip already", slog.String("ip", target))
- c <- []Target{
- {
- Host: target,
- IP: addr,
- },
+ c <- Target{
+ Host: target,
+ IP: addr,
  }
  return
  }
@@ -53,11 +51,9 @@ func (r *Resolver) Do(target string, c chan<- []Target) {
  r.logger.Error("address failed parsing", slog.String("ip", i))
  continue
  }
- ips = append(ips, Target{
+ c <- Target{
  Host: target,
  IP: a,
- })
+ }
  }
-
- c <- ips
 }