refactor: Use package local metrics

Part 2 (#1578) Improve performance of metrics by moving them to the package that needs them. This reduces the overhead to a simple atomic increment for basic counters like cache hits/misses. This also uses `promauto` to avoid the second step of having to register metrics. Signed-off-by: SuperQ <[email protected]>
0xERR0R · Sep 13, 2024 · 9d1d131 · 9d1d131
1 parent 776a8f1
commit 9d1d131
Show file tree

Hide file tree

Showing 8 changed files with 70 additions and 148 deletions.
diff --git a/cmd/serve.go b/cmd/serve.go
@@ -10,9 +10,12 @@ import (
 	"github.com/0xERR0R/blocky/config"
 	"github.com/0xERR0R/blocky/evt"
 	"github.com/0xERR0R/blocky/log"
+	"github.com/0xERR0R/blocky/metrics"
 	"github.com/0xERR0R/blocky/server"
 	"github.com/0xERR0R/blocky/util"
 
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"
 	"github.com/spf13/cobra"
 )
 
@@ -21,6 +24,13 @@ var (
 	done              = make(chan bool, 1)
 	isConfigMandatory = true
 	signals           = make(chan os.Signal, 1)
+
+	versionInfoMetric = promauto.With(metrics.Reg).NewGaugeVec(
+		prometheus.GaugeOpts{
+			Name: "blocky_build_info",
+			Help: "Version number and build info",
+		}, []string{"version", "build_time"},
+	)
 )
 
 func newServeCommand() *cobra.Command {
@@ -76,6 +86,7 @@ func startServer(_ *cobra.Command, _ []string) error {
 	}()
 
 	evt.Bus().Publish(evt.ApplicationStarted, util.Version, util.BuildTime)
+	versionInfoMetric.WithLabelValues(util.Version, util.BuildTime).Set(1)
 	<-done
 
 	return terminationErr

diff --git a/evt/events.go b/evt/events.go
@@ -8,9 +8,6 @@ const (
 	// BlockingEnabledEvent fires if blocking status will be changed. Parameter: boolean (enabled = true)
 	BlockingEnabledEvent = "blocking:enabled"
 
-	// BlockingCacheGroupChanged fires, if a list group is changed. Parameter: list type, group name, element count
-	BlockingCacheGroupChanged = "blocking:cachingGroupChanged"
-
 	// CachingDomainPrefetched fires if a domain will be prefetched, Parameter: domain name
 	CachingDomainPrefetched = "caching:prefetched"
 
@@ -23,9 +20,6 @@ const (
 	// CachingDomainsToPrefetchCountChanged fires, if a number of domains being prefetched changed, Parameter: new count
 	CachingDomainsToPrefetchCountChanged = "caching:domainsToPrefetchCountChanged"
 
-	// CachingFailedDownloadChanged fires, if a download of a blocking list or hosts file fails
-	CachingFailedDownloadChanged = "caching:failedDownload"
-
 	// ApplicationStarted fires on start of the application. Parameter: version number, build time
 	ApplicationStarted = "application:started"
 )

diff --git a/lists/downloader.go b/lists/downloader.go
@@ -9,8 +9,21 @@ import (
 	"net/http"
 
 	"github.com/0xERR0R/blocky/config"
-	"github.com/0xERR0R/blocky/evt"
+	"github.com/0xERR0R/blocky/metrics"
+
 	"github.com/avast/retry-go/v4"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"
+)
+
+//nolint:gochecknoglobals
+var (
+	failedDownloadsTotal = promauto.With(metrics.Reg).NewCounter(
+		prometheus.CounterOpts{
+			Name: "blocky_failed_downloads_total",
+			Help: "Failed download counter",
+		},
+	)
 )
 
 // TransientError represents a temporary error like timeout, network errors...
@@ -112,5 +125,5 @@ func (d *httpDownloader) DownloadFile(ctx context.Context, link string) (io.Read
 }
 
 func onDownloadError(link string) {
-	evt.Bus().Publish(evt.CachingFailedDownloadChanged, link)
+	failedDownloadsTotal.Inc()
 }
diff --git a/lists/downloader_test.go b/lists/downloader_test.go
@@ -12,7 +12,6 @@ import (
 	"time"
 
 	"github.com/0xERR0R/blocky/config"
-	. "github.com/0xERR0R/blocky/evt"
 	. "github.com/0xERR0R/blocky/helpertest"
 	"github.com/0xERR0R/blocky/log"
 	. "github.com/onsi/ginkgo/v2"
@@ -22,27 +21,16 @@ import (
 
 var _ = Describe("Downloader", func() {
 	var (
-		sutConfig                     config.Downloader
-		sut                           *httpDownloader
-		failedDownloadCountEvtChannel chan string
-		loggerHook                    *test.Hook
+		sutConfig  config.Downloader
+		sut        *httpDownloader
+		loggerHook *test.Hook
 	)
 	BeforeEach(func() {
 		var err error
 
 		sutConfig, err = config.WithDefaults[config.Downloader]()
 		Expect(err).Should(Succeed())
 
-		failedDownloadCountEvtChannel = make(chan string, 5)
-		// collect received events in the channel
-		fn := func(url string) {
-			failedDownloadCountEvtChannel <- url
-		}
-		Expect(Bus().Subscribe(CachingFailedDownloadChanged, fn)).Should(Succeed())
-		DeferCleanup(func() {
-			Expect(Bus().Unsubscribe(CachingFailedDownloadChanged, fn)).Should(Succeed())
-		})
-
 		loggerHook = test.NewGlobal()
 		log.Log().AddHook(loggerHook)
 		DeferCleanup(loggerHook.Reset)
@@ -106,8 +94,6 @@ var _ = Describe("Downloader", func() {
 				Expect(err).Should(HaveOccurred())
 				Expect(reader).Should(BeNil())
 				Expect(err.Error()).Should(Equal("got status code 404"))
-				Expect(failedDownloadCountEvtChannel).Should(HaveLen(3))
-				Expect(failedDownloadCountEvtChannel).Should(Receive(Equal(server.URL)))
 			})
 		})
 		When("Wrong URL is defined", func() {
@@ -119,9 +105,6 @@ var _ = Describe("Downloader", func() {
 
 				Expect(err).Should(HaveOccurred())
 				Expect(loggerHook.LastEntry().Message).Should(ContainSubstring("Can't download file: "))
-				// failed download event was emitted only once
-				Expect(failedDownloadCountEvtChannel).Should(HaveLen(1))
-				Expect(failedDownloadCountEvtChannel).Should(Receive(Equal("somewrongurl")))
 			})
 		})
 
@@ -158,9 +141,6 @@ var _ = Describe("Downloader", func() {
 				Expect(err).Should(Succeed())
 				Expect(buf.String()).Should(Equal("blocked1.com"))
 
-				// failed download event was emitted only once
-				Expect(failedDownloadCountEvtChannel).Should(HaveLen(1))
-				Expect(failedDownloadCountEvtChannel).Should(Receive(Equal(server.URL)))
 				Expect(loggerHook.LastEntry().Message).Should(ContainSubstring("Temporary network err / Timeout occurred: "))
 			})
 		})
@@ -184,10 +164,6 @@ var _ = Describe("Downloader", func() {
 					Expect(errors.As(err, new(*TransientError))).Should(BeTrue())
 					Expect(err.Error()).Should(ContainSubstring("Timeout"))
 					Expect(reader).Should(BeNil())
-
-					// failed download event was emitted 3 times
-					Expect(failedDownloadCountEvtChannel).Should(HaveLen(3))
-					Expect(failedDownloadCountEvtChannel).Should(Receive(Equal(server.URL)))
 				})
 		})
 		When("DNS resolution of passed URL fails", func() {
@@ -206,10 +182,6 @@ var _ = Describe("Downloader", func() {
 					var dnsError *net.DNSError
 					Expect(errors.As(err, &dnsError)).Should(BeTrue(), "received error %w", err)
 					Expect(reader).Should(BeNil())
-
-					// failed download event was emitted 3 times
-					Expect(failedDownloadCountEvtChannel).Should(HaveLen(3))
-					Expect(failedDownloadCountEvtChannel).Should(Receive(Equal("http://some.domain.which.does.not.exist")))
 					Expect(loggerHook.LastEntry().Message).Should(ContainSubstring("Name resolution err: "))
 				})
 		})

diff --git a/lists/list_cache.go b/lists/list_cache.go
@@ -6,23 +6,48 @@ import (
 	"errors"
 	"fmt"
 	"net"
-
-	"github.com/sirupsen/logrus"
+	"time"
 
 	"github.com/0xERR0R/blocky/cache/stringcache"
 	"github.com/0xERR0R/blocky/config"
-	"github.com/0xERR0R/blocky/evt"
 	"github.com/0xERR0R/blocky/lists/parsers"
 	"github.com/0xERR0R/blocky/log"
+	"github.com/0xERR0R/blocky/metrics"
+
 	"github.com/ThinkChaos/parcour"
 	"github.com/ThinkChaos/parcour/jobgroup"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"
+	"github.com/sirupsen/logrus"
 )
 
 const (
 	groupProducersBufferCap = 1000
 	regexWarningThreshold   = 500
 )
 
+//nolint:gochecknoglobals
+var (
+	lastListGroupRefreshTimestamp = promauto.With(metrics.Reg).NewGauge(
+		prometheus.GaugeOpts{
+			Name: "blocky_last_list_group_refresh_timestamp_seconds",
+			Help: "Timestamp of last list refresh",
+		},
+	)
+	denylistEntries = promauto.With(metrics.Reg).NewGaugeVec(
+		prometheus.GaugeOpts{
+			Name: "blocky_denylist_cache_entries",
+			Help: "Number of entries in the denylist cache",
+		}, []string{"group"},
+	)
+	allowlistEntries = promauto.With(metrics.Reg).NewGaugeVec(
+		prometheus.GaugeOpts{
+			Name: "blocky_allowlist_cache_entries",
+			Help: "Number of entries in the allowlist cache",
+		}, []string{"group"},
+	)
+)
+
 // ListCacheType represents the type of cached list ENUM(
 // denylist // is a list with blocked domains
 // allowlist // is a list with allowlisted domains / IPs
@@ -144,7 +169,7 @@ func (b *ListCache) refresh(ctx context.Context) error {
 
 			count := b.groupedCache.ElementCount(group)
 
-			evt.Bus().Publish(evt.BlockingCacheGroupChanged, b.listType, group, count)
+			updateGroupMetrics(b.listType, group, count)
 
 			logger().WithFields(logrus.Fields{
 				"group":       group,
@@ -277,3 +302,14 @@ func (b *ListCache) parseFile(ctx context.Context, opener SourceOpener, resultCh
 
 	return nil
 }
+
+func updateGroupMetrics(listType ListCacheType, group string, count int) {
+	lastListGroupRefreshTimestamp.Set(float64(time.Now().Unix()))
+
+	switch listType {
+	case ListCacheTypeDenylist:
+		denylistEntries.WithLabelValues(group).Set(float64(count))
+	case ListCacheTypeAllowlist:
+		allowlistEntries.WithLabelValues(group).Set(float64(count))
+	}
+}
diff --git a/lists/list_cache_test.go b/lists/list_cache_test.go
@@ -11,7 +11,6 @@ import (
 	"strings"
 
 	"github.com/0xERR0R/blocky/config"
-	. "github.com/0xERR0R/blocky/evt"
 	"github.com/0xERR0R/blocky/lists/parsers"
 	"github.com/0xERR0R/blocky/log"
 	"github.com/google/uuid"
@@ -251,25 +250,6 @@ var _ = Describe("ListCache", func() {
 				Expect(group).Should(ContainElement("gr2"))
 			})
 		})
-		When("List will be updated", func() {
-			resultCnt := 0
-
-			BeforeEach(func() {
-				lists = map[string][]config.BytesSource{
-					"gr1": config.NewBytesSources(server1.URL),
-				}
-
-				_ = Bus().SubscribeOnce(BlockingCacheGroupChanged, func(listType ListCacheType, group string, cnt int) {
-					resultCnt = cnt
-				})
-			})
-
-			It("event should be fired and contain count of elements in downloaded lists", func() {
-				group := sut.Match("blocked1.com", []string{})
-				Expect(group).Should(BeEmpty())
-				Expect(resultCnt).Should(Equal(3))
-			})
-		})
 		When("multiple groups are passed", func() {
 			BeforeEach(func() {
 				lists = map[string][]config.BytesSource{