Clean up scraper factory (#656)

The function is becoming too messy. Clean up a little by replacing the
counter arguments with a generic metrics interface.

Signed-off-by: Marcelo E. Magallon <[email protected]>

Author: mem

internal/checks/checks.go

@@ -867,9 +867,15 @@ func (c *Updater) addAndStartScraperWithLock(ctx context.Context, check model.Ch
 		return err
 	}
 
+	metrics := scraper.NewMetrics(
+		scrapeCounter,
+		scraper.NewIncrementerFromCounterVec(scrapeErrorCounter),
+	)
+
 	scraper, err := c.scraperFactory(
-		ctx, check, c.publisher, *c.probe, c.logger, scrapeCounter,
-		scraper.NewIncrementerFromCounterVec(scrapeErrorCounter), c.k6Runner,
+		ctx, check, c.publisher, *c.probe, c.logger,
+		metrics,
+		c.k6Runner,
 		c.tenantLimits, c.telemeter,
 	)
 	if err != nil {

internal/checks/checks_test.go

@@ -296,18 +296,17 @@ func (l testLabelsLimiter) LogLabels(ctx context.Context, tenantID model.GlobalI
 }
 
 func testScraperFactory(ctx context.Context, check model.Check, publisher pusher.Publisher,
-	_ sm.Probe, logger zerolog.Logger, scrapeCounter scraper.Incrementer, scrapeErrorCounter scraper.IncrementerVec,
+	_ sm.Probe, logger zerolog.Logger, metrics scraper.Metrics,
 	k6Runner k6runner.Runner, labelsLimiter scraper.LabelsLimiter, telemeter *telemetry.Telemeter,
 ) (*scraper.Scraper, error) {
 	return scraper.NewWithOpts(
 		ctx,
 		check,
 		scraper.ScraperOpts{
-			ErrorCounter:  scrapeErrorCounter,
 			Logger:        logger,
 			ProbeFactory:  testProbeFactory{},
 			Publisher:     publisher,
-			ScrapeCounter: scrapeCounter,
+			Metrics:       metrics,
 			LabelsLimiter: testLabelsLimiter{},
 			Telemeter:     telemeter,
 		},

internal/scraper/metrics.go

@@ -0,0 +1,47 @@
+package scraper
+
+import "github.com/prometheus/client_golang/prometheus"
+
+func NewMetrics(scrapeCounter Incrementer, errorCounter IncrementerVec) Metrics {
+	return metrics{
+		scrapeCounter: scrapeCounter,
+		errorCounter:  errorCounter,
+	}
+}
+
+type metrics struct {
+	scrapeCounter Incrementer
+	errorCounter  IncrementerVec
+}
+
+func (m metrics) AddScrape() {
+	m.scrapeCounter.Inc()
+}
+
+func (m metrics) AddCheckError() {
+	m.errorCounter.WithLabelValues("check").Inc()
+}
+
+func (m metrics) AddCollectorError() {
+	m.errorCounter.WithLabelValues("collector").Inc()
+}
+
+type Incrementer interface {
+	Inc()
+}
+
+type IncrementerVec interface {
+	WithLabelValues(...string) Incrementer
+}
+
+type counterVecWrapper struct {
+	c *prometheus.CounterVec
+}
+
+func (c *counterVecWrapper) WithLabelValues(v ...string) Incrementer {
+	return c.c.WithLabelValues(v...)
+}
+
+func NewIncrementerFromCounterVec(c *prometheus.CounterVec) IncrementerVec {
+	return &counterVecWrapper{c: c}
+}

internal/scraper/scraper.go

@@ -44,24 +44,10 @@ var (
 	staleMarker float64 = math.Float64frombits(staleNaN)
 )
 
-type Incrementer interface {
-	Inc()
-}
-
-type IncrementerVec interface {
-	WithLabelValues(...string) Incrementer
-}
-
-type counterVecWrapper struct {
-	c *prometheus.CounterVec
-}
-
-func (c *counterVecWrapper) WithLabelValues(v ...string) Incrementer {
-	return c.c.WithLabelValues(v...)
-}
-
-func NewIncrementerFromCounterVec(c *prometheus.CounterVec) IncrementerVec {
-	return &counterVecWrapper{c: c}
+type Metrics interface {
+	AddScrape()
+	AddCheckError()
+	AddCollectorError()
 }
 
 type LabelsLimiter interface {
@@ -84,16 +70,15 @@ type Scraper struct {
 	prober        prober.Prober
 	labelsLimiter LabelsLimiter
 	stop          chan struct{}
-	scrapeCounter Incrementer
-	errorCounter  IncrementerVec
+	metrics       Metrics
 	summaries     map[uint64]prometheus.Summary
 	histograms    map[uint64]prometheus.Histogram
 	telemeter     Telemeter
 }
 
 type Factory func(
 	ctx context.Context, check model.Check, publisher pusher.Publisher, probe sm.Probe, logger zerolog.Logger,
-	scrapeCounter Incrementer, errorCounter IncrementerVec, k6runner k6runner.Runner, labelsLimiter LabelsLimiter,
+	metrics Metrics, k6runner k6runner.Runner, labelsLimiter LabelsLimiter,
 	telemeter *telemetry.Telemeter,
 ) (*Scraper, error)
 
@@ -122,15 +107,14 @@ func (d *probeData) Tenant() model.GlobalID {
 
 func New(
 	ctx context.Context, check model.Check, publisher pusher.Publisher, probe sm.Probe,
-	logger zerolog.Logger, scrapeCounter Incrementer, errorCounter IncrementerVec,
+	logger zerolog.Logger, metrics Metrics,
 	k6runner k6runner.Runner, labelsLimiter LabelsLimiter, telemeter *telemetry.Telemeter,
 ) (*Scraper, error) {
 	return NewWithOpts(ctx, check, ScraperOpts{
 		Probe:         probe,
 		Publisher:     publisher,
 		Logger:        logger,
-		ScrapeCounter: scrapeCounter,
-		ErrorCounter:  errorCounter,
+		Metrics:       metrics,
 		ProbeFactory:  prober.NewProberFactory(k6runner, probe.Id),
 		LabelsLimiter: labelsLimiter,
 		Telemeter:     telemeter,
@@ -141,8 +125,7 @@ type ScraperOpts struct {
 	Probe         sm.Probe
 	Publisher     pusher.Publisher
 	Logger        zerolog.Logger
-	ScrapeCounter Incrementer
-	ErrorCounter  IncrementerVec
+	Metrics       Metrics
 	ProbeFactory  prober.ProberFactory
 	LabelsLimiter LabelsLimiter
 	Telemeter     Telemeter
@@ -179,8 +162,7 @@ func NewWithOpts(ctx context.Context, check model.Check, opts ScraperOpts) (*Scr
 		prober:        smProber,
 		labelsLimiter: opts.LabelsLimiter,
 		stop:          make(chan struct{}),
-		scrapeCounter: opts.ScrapeCounter,
-		errorCounter:  opts.ErrorCounter,
+		metrics:       opts.Metrics,
 		summaries:     make(map[uint64]prometheus.Summary),
 		histograms:    make(map[uint64]prometheus.Histogram),
 		telemeter:     opts.Telemeter,
@@ -265,7 +247,7 @@ type scrapeHandler struct {
 }
 
 func (h *scrapeHandler) scrape(ctx context.Context, t time.Time) {
-	h.scraper.scrapeCounter.Inc()
+	h.scraper.metrics.AddScrape()
 
 	var err error
 
@@ -280,13 +262,13 @@ func (h *scrapeHandler) scrape(ctx context.Context, t time.Time) {
 
 	switch {
 	case errors.Is(err, errCheckFailed):
-		h.scraper.errorCounter.WithLabelValues("check").Inc()
+		h.scraper.metrics.AddCheckError()
 		h.sm.fail(func() {
 			h.scraper.logger.Info().Msg("check entered FAIL state")
 		})
 
 	case err != nil:
-		h.scraper.errorCounter.WithLabelValues("collector").Inc()
+		h.scraper.metrics.AddCollectorError()
 		h.scraper.logger.Error().Err(err).Msg("error collecting data")
 		return
 

internal/scraper/scraper_test.go

@@ -1610,12 +1610,13 @@ func TestScraperRun(t *testing.T) {
 	testProber := &testProberB{wantedFailures: 2}
 	testTelemeter := &testTelemeter{}
 
+	metrics := NewMetrics(&counter, &errCounter)
+
 	s, err := NewWithOpts(ctx, check, ScraperOpts{
-		ScrapeCounter: &counter,
-		ErrorCounter:  &errCounter,
-		ProbeFactory:  testProbeFactory{builder: func() prober.Prober { return testProber }},
-		Logger:        zerolog.New(zerolog.NewTestWriter(t)),
-		Publisher:     &testPublisher{},
+		Metrics:      metrics,
+		ProbeFactory: testProbeFactory{builder: func() prober.Prober { return testProber }},
+		Logger:       zerolog.New(zerolog.NewTestWriter(t)),
+		Publisher:    &testPublisher{},
 		LabelsLimiter: testLabelsLimiter{
 			maxMetricLabels: 20,
 			maxLogLabels:    15,