Merge release-2.32 forward into main (#10032)

* storage: expose bug in iterators #10027

Signed-off-by: beorn7 <beorn@grafana.com>

* storage: fix bug #10027 in iterators' Seek method

Signed-off-by: beorn7 <beorn@grafana.com>

* Append reporting metrics without limit

If reporting metrics fails due to reaching the limit, this makes the
target appear as UP in the UI, but the metrics are missing.

This commit bypasses that limit for report metrics.

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>

* Remove check against cfg so interval/ timeout are always set (#10023) (#10031)

Signed-off-by: Nicholas Blott <blottn@tcd.ie>

Co-authored-by: Nicholas Blott <blottn@tcd.ie>

* Cut v2.32.1

Signed-off-by: Julius Volz <julius.volz@gmail.com>

* Apply suggestions from code review

Signed-off-by: Julius Volz <julius.volz@gmail.com>

Co-authored-by: Levi Harrison <git@leviharrison.dev>

Co-authored-by: Julien Pivotto <roidelapluie@inuits.eu>
Co-authored-by: Nicholas Blott <blottn@tcd.ie>
Co-authored-by: Julius Volz <julius.volz@gmail.com>
Co-authored-by: Levi Harrison <git@leviharrison.dev>

CHANGELOG.md

@@ -1,3 +1,9 @@
+## 2.32.1 / 2021-12-17
+
+* [BUGFIX] Scrape: Fix reporting metrics when sample limit is reached during the report. #9996
+* [BUGFIX] Scrape: Ensure that scrape interval and scrape timeout are always set. #10023
+* [BUGFIX] TSDB: Expose and fix bug in iterators' `Seek()` method. #10030
+
 ## 2.32.0 / 2021-12-09
 
 This release introduces the Prometheus Agent, a new mode of operation for

VERSION

@@ -1 +1 @@
-2.32.0
+2.32.1

scrape/scrape.go

@@ -305,7 +305,7 @@ func newScrapePool(cfg *config.ScrapeConfig, app storage.Appendable, jitterSeed
 				return mutateSampleLabels(l, opts.target, opts.honorLabels, opts.mrc)
 			},
 			func(l labels.Labels) labels.Labels { return mutateReportSampleLabels(l, opts.target) },
-			func(ctx context.Context) storage.Appender { return appender(app.Appender(ctx), opts.sampleLimit) },
+			func(ctx context.Context) storage.Appender { return app.Appender(ctx) },
 			cache,
 			jitterSeed,
 			opts.honorTimestamps,
@@ -1426,6 +1426,9 @@ func (sl *scrapeLoop) append(app storage.Appender, b []byte, contentType string,
 		e              exemplar.Exemplar // escapes to heap so hoisted out of loop
 	)
 
+	// Take an appender with limits.
+	app = appender(app, sl.sampleLimit)
+
 	defer func() {
 		if err != nil {
 			return

scrape/scrape_test.go

@@ -463,7 +463,7 @@ func TestScrapePoolAppender(t *testing.T) {
 	appl, ok := loop.(*scrapeLoop)
 	require.True(t, ok, "Expected scrapeLoop but got %T", loop)
 
-	wrapped := appl.appender(context.Background())
+	wrapped := appender(appl.appender(context.Background()), 0)
 
 	tl, ok := wrapped.(*timeLimitAppender)
 	require.True(t, ok, "Expected timeLimitAppender but got %T", wrapped)
@@ -471,20 +471,21 @@ func TestScrapePoolAppender(t *testing.T) {
 	_, ok = tl.Appender.(nopAppender)
 	require.True(t, ok, "Expected base appender but got %T", tl.Appender)
 
+	sampleLimit := 100
 	loop = sp.newLoop(scrapeLoopOptions{
 		target:      &Target{},
-		sampleLimit: 100,
+		sampleLimit: sampleLimit,
 	})
 	appl, ok = loop.(*scrapeLoop)
 	require.True(t, ok, "Expected scrapeLoop but got %T", loop)
 
-	wrapped = appl.appender(context.Background())
+	wrapped = appender(appl.appender(context.Background()), sampleLimit)
 
 	sl, ok := wrapped.(*limitAppender)
 	require.True(t, ok, "Expected limitAppender but got %T", wrapped)
 
 	tl, ok = sl.Appender.(*timeLimitAppender)
-	require.True(t, ok, "Expected limitAppender but got %T", sl.Appender)
+	require.True(t, ok, "Expected timeLimitAppender but got %T", sl.Appender)
 
 	_, ok = tl.Appender.(nopAppender)
 	require.True(t, ok, "Expected base appender but got %T", tl.Appender)
@@ -2772,6 +2773,72 @@ func TestScrapeReportSingleAppender(t *testing.T) {
 	}
 }
 
+func TestScrapeReportLimit(t *testing.T) {
+	s := teststorage.New(t)
+	defer s.Close()
+
+	cfg := &config.ScrapeConfig{
+		JobName:        "test",
+		SampleLimit:    5,
+		Scheme:         "http",
+		ScrapeInterval: model.Duration(100 * time.Millisecond),
+		ScrapeTimeout:  model.Duration(100 * time.Millisecond),
+	}
+
+	var (
+		scrapes      int
+		scrapedTwice = make(chan bool)
+	)
+
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprint(w, "metric_a 44\nmetric_b 44\nmetric_c 44\nmetric_d 44\n")
+		scrapes++
+		if scrapes == 2 {
+			close(scrapedTwice)
+		}
+	}))
+	defer ts.Close()
+
+	sp, err := newScrapePool(cfg, s, 0, nil, false)
+	require.NoError(t, err)
+	defer sp.stop()
+
+	testURL, err := url.Parse(ts.URL)
+	require.NoError(t, err)
+	sp.Sync([]*targetgroup.Group{
+		{
+			Targets: []model.LabelSet{{model.AddressLabel: model.LabelValue(testURL.Host)}},
+		},
+	})
+
+	select {
+	case <-time.After(5 * time.Second):
+		t.Fatalf("target was not scraped twice")
+	case <-scrapedTwice:
+		// If the target has been scraped twice, report samples from the first
+		// scrape have been inserted in the database.
+	}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	q, err := s.Querier(ctx, time.Time{}.UnixNano(), time.Now().UnixNano())
+	require.NoError(t, err)
+	defer q.Close()
+	series := q.Select(false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "up"))
+
+	var found bool
+	for series.Next() {
+		i := series.At().Iterator()
+		for i.Next() {
+			_, v := i.At()
+			require.Equal(t, 1.0, v)
+			found = true
+		}
+	}
+
+	require.True(t, found)
+}
+
 func TestScrapeLoopLabelLimit(t *testing.T) {
 	tests := []struct {
 		title           string

storage/remote/codec.go

@@ -360,6 +360,9 @@ func (c *concreteSeriesIterator) Seek(t int64) bool {
 	if c.cur == -1 {
 		c.cur = 0
 	}
+	if c.cur >= len(c.series.samples) {
+		return false
+	}
 	// No-op check.
 	if s := c.series.samples[c.cur]; s.Timestamp >= t {
 		return true

storage/remote/codec_test.go

@@ -233,6 +233,11 @@ func TestConcreteSeriesIterator(t *testing.T) {
 	ts, v = it.At()
 	require.Equal(t, int64(3), ts)
 	require.Equal(t, 3., v)
+
+	// Seek beyond the end.
+	require.False(t, it.Seek(5))
+	// And we don't go back. (This exposes issue #10027.)
+	require.False(t, it.Seek(2))
 }
 
 func TestFromQueryResultWithDuplicates(t *testing.T) {

storage/series.go

@@ -99,6 +99,9 @@ func (it *listSeriesIterator) Seek(t int64) bool {
 	if it.idx == -1 {
 		it.idx = 0
 	}
+	if it.idx >= it.samples.Len() {
+		return false
+	}
 	// No-op check.
 	if s := it.samples.Get(it.idx); s.T() >= t {
 		return true

storage/series_test.go

@@ -51,4 +51,9 @@ func TestListSeriesIterator(t *testing.T) {
 	ts, v = it.At()
 	require.Equal(t, int64(3), ts)
 	require.Equal(t, 3., v)
+
+	// Seek beyond the end.
+	require.False(t, it.Seek(5))
+	// And we don't go back. (This exposes issue #10027.)
+	require.False(t, it.Seek(2))
 }

tsdb/tsdbutil/buffer_test.go

@@ -159,6 +159,9 @@ func (it *listSeriesIterator) Seek(t int64) bool {
 	if it.idx == -1 {
 		it.idx = 0
 	}
+	if it.idx >= len(it.list) {
+		return false
+	}
 	// No-op check.
 	if s := it.list[it.idx]; s.T() >= t {
 		return true