Merge pull request #12737 from prometheus/beorn7/histogram

textparse: fix infinite loop during exemplar parsing

model/textparse/protobufparse.go

@@ -56,6 +56,10 @@ type ProtobufParser struct {
 	fieldsDone  bool // true if no more fields of a Summary or (legacy) Histogram to be processed.
 	redoClassic bool // true after parsing a native histogram if we need to parse it again as a classic histogram.
 
+	// exemplarReturned is set to true each time an exemplar has been
+	// returned, and set back to false upon each Next() call.
+	exemplarReturned bool
+
 	// state is marked by the entry we are processing. EntryInvalid implies
 	// that we have to decode the next MetricFamily.
 	state Entry
@@ -293,8 +297,12 @@ func (p *ProtobufParser) Metric(l *labels.Labels) string {
 // Exemplar writes the exemplar of the current sample into the passed
 // exemplar. It returns if an exemplar exists or not. In case of a native
 // histogram, the legacy bucket section is still used for exemplars. To ingest
-// all examplars, call the Exemplar method repeatedly until it returns false.
+// all exemplars, call the Exemplar method repeatedly until it returns false.
 func (p *ProtobufParser) Exemplar(ex *exemplar.Exemplar) bool {
+	if p.exemplarReturned && p.state == EntrySeries {
+		// We only ever return one exemplar per (non-native-histogram) series.
+		return false
+	}
 	m := p.mf.GetMetric()[p.metricPos]
 	var exProto *dto.Exemplar
 	switch p.mf.GetType() {
@@ -335,6 +343,7 @@ func (p *ProtobufParser) Exemplar(ex *exemplar.Exemplar) bool {
 	}
 	p.builder.Sort()
 	ex.Labels = p.builder.Labels()
+	p.exemplarReturned = true
 	return true
 }
 
@@ -342,6 +351,7 @@ func (p *ProtobufParser) Exemplar(ex *exemplar.Exemplar) bool {
 // text format parser). It returns (EntryInvalid, io.EOF) if no samples were
 // read.
 func (p *ProtobufParser) Next() (Entry, error) {
+	p.exemplarReturned = false
 	switch p.state {
 	case EntryInvalid:
 		p.metricPos = 0

model/textparse/protobufparse_test.go

@@ -1779,6 +1779,7 @@ func TestProtobufParse(t *testing.T) {
 					} else {
 						require.Equal(t, true, found, "i: %d", i)
 						require.Equal(t, exp[i].e[0], e, "i: %d", i)
+						require.False(t, p.Exemplar(&e), "too many exemplars returned, i: %d", i)
 					}
 
 				case EntryHistogram:

scrape/scrape_test.go

@@ -1985,13 +1985,14 @@ func TestScrapeLoopAppendNoStalenessIfTimestamp(t *testing.T) {
 
 func TestScrapeLoopAppendExemplar(t *testing.T) {
 	tests := []struct {
-		title           string
-		scrapeText      string
-		contentType     string
-		discoveryLabels []string
-		floats          []floatSample
-		histograms      []histogramSample
-		exemplars       []exemplar.Exemplar
+		title                   string
+		scrapeClassicHistograms bool
+		scrapeText              string
+		contentType             string
+		discoveryLabels         []string
+		floats                  []floatSample
+		histograms              []histogramSample
+		exemplars               []exemplar.Exemplar
 	}{
 		{
 			title:           "Metric without exemplars",
@@ -2145,6 +2146,115 @@ metric: <
 				{Labels: labels.FromStrings("dummyID", "5617"), Value: -0.00029, Ts: 1234568, HasTs: false},
 			},
 		},
+		{
+			title: "Native histogram with two exemplars scraped as classic histogram",
+			scrapeText: `name: "test_histogram"
+help: "Test histogram with many buckets removed to keep it manageable in size."
+type: HISTOGRAM
+metric: <
+  histogram: <
+    sample_count: 175
+    sample_sum: 0.0008280461746287094
+    bucket: <
+      cumulative_count: 2
+      upper_bound: -0.0004899999999999998
+    >
+    bucket: <
+      cumulative_count: 4
+      upper_bound: -0.0003899999999999998
+      exemplar: <
+        label: <
+          name: "dummyID"
+          value: "59727"
+        >
+        value: -0.00039
+        timestamp: <
+          seconds: 1625851155
+          nanos: 146848499
+        >
+      >
+    >
+    bucket: <
+      cumulative_count: 16
+      upper_bound: -0.0002899999999999998
+      exemplar: <
+        label: <
+          name: "dummyID"
+          value: "5617"
+        >
+        value: -0.00029
+      >
+    >
+    schema: 3
+    zero_threshold: 2.938735877055719e-39
+    zero_count: 2
+    negative_span: <
+      offset: -162
+      length: 1
+    >
+    negative_span: <
+      offset: 23
+      length: 4
+    >
+    negative_delta: 1
+    negative_delta: 3
+    negative_delta: -2
+    negative_delta: -1
+    negative_delta: 1
+    positive_span: <
+      offset: -161
+      length: 1
+    >
+    positive_span: <
+      offset: 8
+      length: 3
+    >
+    positive_delta: 1
+    positive_delta: 2
+    positive_delta: -1
+    positive_delta: -1
+  >
+  timestamp_ms: 1234568
+>
+
+`,
+			scrapeClassicHistograms: true,
+			contentType:             "application/vnd.google.protobuf",
+			floats: []floatSample{
+				{metric: labels.FromStrings("__name__", "test_histogram_count"), t: 1234568, f: 175},
+				{metric: labels.FromStrings("__name__", "test_histogram_sum"), t: 1234568, f: 0.0008280461746287094},
+				{metric: labels.FromStrings("__name__", "test_histogram_bucket", "le", "-0.0004899999999999998"), t: 1234568, f: 2},
+				{metric: labels.FromStrings("__name__", "test_histogram_bucket", "le", "-0.0003899999999999998"), t: 1234568, f: 4},
+				{metric: labels.FromStrings("__name__", "test_histogram_bucket", "le", "-0.0002899999999999998"), t: 1234568, f: 16},
+				{metric: labels.FromStrings("__name__", "test_histogram_bucket", "le", "+Inf"), t: 1234568, f: 175},
+			},
+			histograms: []histogramSample{{
+				t: 1234568,
+				h: &histogram.Histogram{
+					Count:         175,
+					ZeroCount:     2,
+					Sum:           0.0008280461746287094,
+					ZeroThreshold: 2.938735877055719e-39,
+					Schema:        3,
+					PositiveSpans: []histogram.Span{
+						{Offset: -161, Length: 1},
+						{Offset: 8, Length: 3},
+					},
+					NegativeSpans: []histogram.Span{
+						{Offset: -162, Length: 1},
+						{Offset: 23, Length: 4},
+					},
+					PositiveBuckets: []int64{1, 2, -1, -1},
+					NegativeBuckets: []int64{1, 3, -2, -1, 1},
+				},
+			}},
+			exemplars: []exemplar.Exemplar{
+				{Labels: labels.FromStrings("dummyID", "59727"), Value: -0.00039, Ts: 1625851155146, HasTs: true},
+				{Labels: labels.FromStrings("dummyID", "5617"), Value: -0.00029, Ts: 1234568, HasTs: false},
+				{Labels: labels.FromStrings("dummyID", "59727"), Value: -0.00039, Ts: 1625851155146, HasTs: true},
+				{Labels: labels.FromStrings("dummyID", "5617"), Value: -0.00029, Ts: 1234568, HasTs: false},
+			},
+		},
 	}
 
 	for _, test := range tests {
@@ -2171,7 +2281,7 @@ metric: <
 				nil,
 				0,
 				0,
-				false,
+				test.scrapeClassicHistograms,
 				false,
 				false,
 				nil,
@@ -2181,6 +2291,9 @@ metric: <
 			now := time.Now()
 
 			for i := range test.floats {
+				if test.floats[i].t != 0 {
+					continue
+				}
 				test.floats[i].t = timestamp.FromTime(now)
 			}