From 04ce817c49262cb87e858b32d7d6aaf518940c23 Mon Sep 17 00:00:00 2001
From: Julien Pivotto <roidelapluie@gmail.com>
Date: Tue, 12 Mar 2019 11:26:18 +0100
Subject: [PATCH] scrape: Rewrite scrape loop options as a struct (#5314)

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
---
 scrape/manager_test.go |  2 +-
 scrape/scrape.go       | 42 ++++++++++++++++++++++++++++++++----------
 scrape/scrape_test.go  | 15 ++++++++++-----
 3 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/scrape/manager_test.go b/scrape/manager_test.go
index 499d13885..70c681b93 100644
--- a/scrape/manager_test.go
+++ b/scrape/manager_test.go
@@ -274,7 +274,7 @@ scrape_configs:
 	)
 
 	scrapeManager := NewManager(nil, nil)
-	newLoop := func(_ *Target, s scraper, _ int, _ bool, _ []*relabel.Config) loop {
+	newLoop := func(scrapeLoopOptions) loop {
 		ch <- struct{}{}
 		return noopLoop()
 	}
diff --git a/scrape/scrape.go b/scrape/scrape.go
index d21bc04bc..668c2afc5 100644
--- a/scrape/scrape.go
+++ b/scrape/scrape.go
@@ -158,7 +158,15 @@ type scrapePool struct {
 	cancel         context.CancelFunc
 
 	// Constructor for new scrape loops. This is settable for testing convenience.
-	newLoop func(*Target, scraper, int, bool, []*relabel.Config) loop
+	newLoop func(scrapeLoopOptions) loop
+}
+
+type scrapeLoopOptions struct {
+	target      *Target
+	scraper     scraper
+	limit       int
+	honorLabels bool
+	mrc         []*relabel.Config
 }
 
 const maxAheadTime = 10 * time.Minute
@@ -189,24 +197,26 @@ func newScrapePool(cfg *config.ScrapeConfig, app Appendable, logger log.Logger)
 		loops:         map[uint64]loop{},
 		logger:        logger,
 	}
-	sp.newLoop = func(t *Target, s scraper, limit int, honor bool, mrc []*relabel.Config) loop {
+	sp.newLoop = func(opts scrapeLoopOptions) loop {
 		// Update the targets retrieval function for metadata to a new scrape cache.
 		cache := newScrapeCache()
-		t.setMetadataStore(cache)
+		opts.target.setMetadataStore(cache)
 
 		return newScrapeLoop(
 			ctx,
-			s,
-			log.With(logger, "target", t),
+			opts.scraper,
+			log.With(logger, "target", opts.target),
 			buffers,
-			func(l labels.Labels) labels.Labels { return mutateSampleLabels(l, t, honor, mrc) },
-			func(l labels.Labels) labels.Labels { return mutateReportSampleLabels(l, t) },
+			func(l labels.Labels) labels.Labels {
+				return mutateSampleLabels(l, opts.target, opts.honorLabels, opts.mrc)
+			},
+			func(l labels.Labels) labels.Labels { return mutateReportSampleLabels(l, opts.target) },
 			func() storage.Appender {
 				app, err := app.Appender()
 				if err != nil {
 					panic(err)
 				}
-				return appender(app, limit)
+				return appender(app, opts.limit)
 			},
 			cache,
 		)
@@ -285,7 +295,13 @@ func (sp *scrapePool) reload(cfg *config.ScrapeConfig) error {
 		var (
 			t       = sp.activeTargets[fp]
 			s       = &targetScraper{Target: t, client: sp.client, timeout: timeout}
-			newLoop = sp.newLoop(t, s, limit, honor, mrc)
+			newLoop = sp.newLoop(scrapeLoopOptions{
+				target:      t,
+				scraper:     s,
+				limit:       limit,
+				honorLabels: honor,
+				mrc:         mrc,
+			})
 		)
 		wg.Add(1)
 
@@ -360,7 +376,13 @@ func (sp *scrapePool) sync(targets []*Target) {
 
 		if _, ok := sp.activeTargets[hash]; !ok {
 			s := &targetScraper{Target: t, client: sp.client, timeout: timeout}
-			l := sp.newLoop(t, s, limit, honor, mrc)
+			l := sp.newLoop(scrapeLoopOptions{
+				target:      t,
+				scraper:     s,
+				limit:       limit,
+				honorLabels: honor,
+				mrc:         mrc,
+			})
 
 			sp.activeTargets[hash] = t
 			sp.loops[hash] = l
diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go
index c301fdbb7..23ec8deb7 100644
--- a/scrape/scrape_test.go
+++ b/scrape/scrape_test.go
@@ -219,7 +219,7 @@ func TestScrapePoolReload(t *testing.T) {
 	}
 	// On starting to run, new loops created on reload check whether their preceding
 	// equivalents have been stopped.
-	newLoop := func(_ *Target, s scraper, _ int, _ bool, _ []*relabel.Config) loop {
+	newLoop := func(opts scrapeLoopOptions) loop {
 		l := &testLoop{}
 		l.startFunc = func(interval, timeout time.Duration, errc chan<- error) {
 			if interval != 3*time.Second {
@@ -229,8 +229,8 @@ func TestScrapePoolReload(t *testing.T) {
 				t.Errorf("Expected scrape timeout %d but got %d", 2*time.Second, timeout)
 			}
 			mtx.Lock()
-			if !stopped[s.(*targetScraper).hash()] {
-				t.Errorf("Scrape loop for %v not stopped yet", s.(*targetScraper))
+			if !stopped[opts.scraper.(*targetScraper).hash()] {
+				t.Errorf("Scrape loop for %v not stopped yet", opts.scraper.(*targetScraper))
 			}
 			mtx.Unlock()
 		}
@@ -307,7 +307,9 @@ func TestScrapePoolAppender(t *testing.T) {
 	app := &nopAppendable{}
 	sp, _ := newScrapePool(cfg, app, nil)
 
-	loop := sp.newLoop(&Target{}, nil, 0, false, nil)
+	loop := sp.newLoop(scrapeLoopOptions{
+		target: &Target{},
+	})
 	appl, ok := loop.(*scrapeLoop)
 	if !ok {
 		t.Fatalf("Expected scrapeLoop but got %T", loop)
@@ -322,7 +324,10 @@ func TestScrapePoolAppender(t *testing.T) {
 		t.Fatalf("Expected base appender but got %T", tl.Appender)
 	}
 
-	loop = sp.newLoop(&Target{}, nil, 100, false, nil)
+	loop = sp.newLoop(scrapeLoopOptions{
+		target: &Target{},
+		limit:  100,
+	})
 	appl, ok = loop.(*scrapeLoop)
 	if !ok {
 		t.Fatalf("Expected scrapeLoop but got %T", loop)