textparse: Refactored main testing utils for reusability; fixed proto Units. (#15095)

Signed-off-by: bwplotka <bwplotka@gmail.com>

model/textparse/interface.go

@@ -69,6 +69,8 @@ type Parser interface {
 	// CreatedTimestamp returns the created timestamp (in milliseconds) for the
 	// current sample. It returns nil if it is unknown e.g. if it wasn't set,
 	// if the scrape protocol or metric type does not support created timestamps.
+	// Assume the CreatedTimestamp returned pointer is only valid until
+	// the Next iteration.
 	CreatedTimestamp() *int64
 
 	// Next advances the parser to the next sample.

model/textparse/interface_test.go

@@ -14,11 +14,18 @@
 package textparse
 
 import (
+	"errors"
+	"io"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/prometheus/common/model"
 	"github.com/stretchr/testify/require"
 
+	"github.com/prometheus/prometheus/model/exemplar"
+	"github.com/prometheus/prometheus/model/histogram"
 	"github.com/prometheus/prometheus/model/labels"
+	"github.com/prometheus/prometheus/util/testutil"
 )
 
 func TestNewParser(t *testing.T) {
@@ -103,3 +110,93 @@ func TestNewParser(t *testing.T) {
 		})
 	}
 }
+
+// parsedEntry represents data that is parsed for each entry.
+type parsedEntry struct {
+	// In all but EntryComment, EntryInvalid.
+	m string
+
+	// In EntryHistogram.
+	shs *histogram.Histogram
+	fhs *histogram.FloatHistogram
+
+	// In EntrySeries.
+	v float64
+
+	// In EntrySeries and EntryHistogram.
+	lset labels.Labels
+	t    *int64
+	es   []exemplar.Exemplar
+	ct   *int64
+
+	// In EntryType.
+	typ model.MetricType
+	// In EntryHelp.
+	help string
+	// In EntryUnit.
+	unit string
+	// In EntryComment.
+	comment string
+}
+
+func requireEntries(t *testing.T, exp, got []parsedEntry) {
+	t.Helper()
+
+	testutil.RequireEqualWithOptions(t, exp, got, []cmp.Option{
+		cmp.AllowUnexported(parsedEntry{}),
+	})
+}
+
+func testParse(t *testing.T, p Parser) (ret []parsedEntry) {
+	t.Helper()
+
+	for {
+		et, err := p.Next()
+		if errors.Is(err, io.EOF) {
+			break
+		}
+		require.NoError(t, err)
+
+		var got parsedEntry
+		var m []byte
+		switch et {
+		case EntryInvalid:
+			t.Fatal("entry invalid not expected")
+		case EntrySeries, EntryHistogram:
+			if et == EntrySeries {
+				m, got.t, got.v = p.Series()
+				got.m = string(m)
+			} else {
+				m, got.t, got.shs, got.fhs = p.Histogram()
+				got.m = string(m)
+			}
+
+			p.Metric(&got.lset)
+			for e := (exemplar.Exemplar{}); p.Exemplar(&e); {
+				got.es = append(got.es, e)
+			}
+			// Parser reuses int pointer.
+			if ct := p.CreatedTimestamp(); ct != nil {
+				got.ct = int64p(*ct)
+			}
+		case EntryType:
+			m, got.typ = p.Type()
+			got.m = string(m)
+
+		case EntryHelp:
+			m, h := p.Help()
+			got.m = string(m)
+			got.help = string(h)
+
+		case EntryUnit:
+			m, u := p.Unit()
+			got.m = string(m)
+			got.unit = string(u)
+
+		case EntryComment:
+			got.comment = string(p.Comment())
+		}
+		ret = append(ret, got)
+	}
+	return ret
+}

model/textparse/openmetricsparse_test.go

@@ -14,7 +14,6 @@
 package textparse
 
 import (
-	"errors"
 	"io"
 	"testing"
 
@@ -115,7 +114,7 @@ foobar{quantile="0.99"} 150.1`
 	input += "\nnull_byte_metric{a=\"abc\x00\"} 1"
 	input += "\n# EOF\n"
 
-	exp := []expectedParse{
+	exp := []parsedEntry{
 		{
 			m:    "go_gc_duration_seconds",
 			help: "A summary of the GC invocation durations.",
@@ -190,12 +189,16 @@ foobar{quantile="0.99"} 150.1`
 			m:    `hhh_bucket{le="+Inf"}`,
 			v:    1,
 			lset: labels.FromStrings("__name__", "hhh_bucket", "le", "+Inf"),
-			e:    &exemplar.Exemplar{Labels: labels.FromStrings("id", "histogram-bucket-test"), Value: 4},
+			es: []exemplar.Exemplar{
+				{Labels: labels.FromStrings("id", "histogram-bucket-test"), Value: 4},
+			},
 		}, {
 			m:    `hhh_count`,
 			v:    1,
 			lset: labels.FromStrings("__name__", "hhh_count"),
-			e:    &exemplar.Exemplar{Labels: labels.FromStrings("id", "histogram-count-test"), Value: 4},
+			es: []exemplar.Exemplar{
+				{Labels: labels.FromStrings("id", "histogram-count-test"), Value: 4},
+			},
 		}, {
 			m:   "ggh",
 			typ: model.MetricTypeGaugeHistogram,
@@ -203,12 +206,16 @@ foobar{quantile="0.99"} 150.1`
 			m:    `ggh_bucket{le="+Inf"}`,
 			v:    1,
 			lset: labels.FromStrings("__name__", "ggh_bucket", "le", "+Inf"),
-			e:    &exemplar.Exemplar{Labels: labels.FromStrings("id", "gaugehistogram-bucket-test", "xx", "yy"), Value: 4, HasTs: true, Ts: 123123},
+			es: []exemplar.Exemplar{
+				{Labels: labels.FromStrings("id", "gaugehistogram-bucket-test", "xx", "yy"), Value: 4, HasTs: true, Ts: 123123},
+			},
 		}, {
 			m:    `ggh_count`,
 			v:    1,
 			lset: labels.FromStrings("__name__", "ggh_count"),
-			e:    &exemplar.Exemplar{Labels: labels.FromStrings("id", "gaugehistogram-count-test", "xx", "yy"), Value: 4, HasTs: true, Ts: 123123},
+			es: []exemplar.Exemplar{
+				{Labels: labels.FromStrings("id", "gaugehistogram-count-test", "xx", "yy"), Value: 4, HasTs: true, Ts: 123123},
+			},
 		}, {
 			m:   "smr_seconds",
 			typ: model.MetricTypeSummary,
@@ -216,12 +223,16 @@ foobar{quantile="0.99"} 150.1`
 			m:    `smr_seconds_count`,
 			v:    2,
 			lset: labels.FromStrings("__name__", "smr_seconds_count"),
-			e:    &exemplar.Exemplar{Labels: labels.FromStrings("id", "summary-count-test"), Value: 1, HasTs: true, Ts: 123321},
+			es: []exemplar.Exemplar{
+				{Labels: labels.FromStrings("id", "summary-count-test"), Value: 1, HasTs: true, Ts: 123321},
+			},
 		}, {
 			m:    `smr_seconds_sum`,
 			v:    42,
 			lset: labels.FromStrings("__name__", "smr_seconds_sum"),
-			e:    &exemplar.Exemplar{Labels: labels.FromStrings("id", "summary-sum-test"), Value: 1, HasTs: true, Ts: 123321},
+			es: []exemplar.Exemplar{
+				{Labels: labels.FromStrings("id", "summary-sum-test"), Value: 1, HasTs: true, Ts: 123321},
+			},
 		}, {
 			m:   "ii",
 			typ: model.MetricTypeInfo,
@@ -270,15 +281,19 @@ foobar{quantile="0.99"} 150.1`
 			v:    17,
 			lset: labels.FromStrings("__name__", "foo_total"),
 			t:    int64p(1520879607789),
-			e:    &exemplar.Exemplar{Labels: labels.FromStrings("id", "counter-test"), Value: 5},
+			es: []exemplar.Exemplar{
-			ct:   int64p(1520872607123),
+				{Labels: labels.FromStrings("id", "counter-test"), Value: 5},
+			},
+			ct: int64p(1520872607123),
 		}, {
 			m:    `foo_total{a="b"}`,
 			v:    17.0,
 			lset: labels.FromStrings("__name__", "foo_total", "a", "b"),
 			t:    int64p(1520879607789),
-			e:    &exemplar.Exemplar{Labels: labels.FromStrings("id", "counter-test"), Value: 5},
+			es: []exemplar.Exemplar{
-			ct:   int64p(1520872607123),
+				{Labels: labels.FromStrings("id", "counter-test"), Value: 5},
+			},
+			ct: int64p(1520872607123),
 		}, {
 			m:    "bar",
 			help: "Summary with CT at the end, making sure we find CT even if it's multiple lines a far",
@@ -430,7 +445,8 @@ foobar{quantile="0.99"} 150.1`
 	}
 
 	p := NewOpenMetricsParser([]byte(input), labels.NewSymbolTable(), WithOMParserCTSeriesSkipped())
-	checkParseResultsWithCT(t, p, exp, true)
+	got := testParse(t, p)
+	requireEntries(t, exp, got)
 }
 
 func TestUTF8OpenMetricsParse(t *testing.T) {
@@ -455,7 +471,7 @@ func TestUTF8OpenMetricsParse(t *testing.T) {
 
 	input += "\n# EOF\n"
 
-	exp := []expectedParse{
+	exp := []parsedEntry{
 		{
 			m:    "go.gc_duration_seconds",
 			help: "A summary of the GC invocation durations.",
@@ -504,7 +520,8 @@ choices}`, "strange©™\n'quoted' \"name\"", "6"),
 	}
 
 	p := NewOpenMetricsParser([]byte(input), labels.NewSymbolTable(), WithOMParserCTSeriesSkipped())
-	checkParseResultsWithCT(t, p, exp, true)
+	got := testParse(t, p)
+	requireEntries(t, exp, got)
 }
 
 func TestOpenMetricsParseErrors(t *testing.T) {
@@ -878,8 +895,8 @@ func TestOMNullByteHandling(t *testing.T) {
 	}
 }
 
-// While not desirable, there are cases were CT fails to parse and
+// TestCTParseFailures tests known failure edge cases, we know does not work due
-// these tests show them.
+// current OM spec limitations or clients with broken OM format.
 // TODO(maniktherana): Make sure OM 1.1/2.0 pass CT via metadata or exemplar-like to avoid this.
 func TestCTParseFailures(t *testing.T) {
 	input := `# HELP thing Histogram with _created as first line
@@ -892,68 +909,37 @@ thing_bucket{le="+Inf"} 17`
 
 	input += "\n# EOF\n"
 
-	int64p := func(x int64) *int64 { return &x }
+	exp := []parsedEntry{
-
-	type expectCT struct {
-		m     string
-		ct    *int64
-		typ   model.MetricType
-		help  string
-		isErr bool
-	}
-
-	exp := []expectCT{
 		{
-			m:     "thing",
+			m:    "thing",
-			help:  "Histogram with _created as first line",
+			help: "Histogram with _created as first line",
-			isErr: false,
 		}, {
-			m:     "thing",
+			m:   "thing",
-			typ:   model.MetricTypeHistogram,
+			typ: model.MetricTypeHistogram,
-			isErr: false,
 		}, {
-			m:     `thing_count`,
+			m:  `thing_count`,
-			ct:    int64p(1520872607123),
+			ct: nil, // Should be int64p(1520872607123).
-			isErr: true,
 		}, {
-			m:     `thing_sum`,
+			m:  `thing_sum`,
-			ct:    int64p(1520872607123),
+			ct: nil, // Should be int64p(1520872607123).
-			isErr: true,
 		}, {
-			m:     `thing_bucket{le="0.0"}`,
+			m:  `thing_bucket{le="0.0"}`,
-			ct:    int64p(1520872607123),
+			ct: nil, // Should be int64p(1520872607123).
-			isErr: true,
 		}, {
-			m:     `thing_bucket{le="+Inf"}`,
+			m:  `thing_bucket{le="+Inf"}`,
-			ct:    int64p(1520872607123),
+			ct: nil, // Should be int64p(1520872607123),
-			isErr: true,
 		},
 	}
 
 	p := NewOpenMetricsParser([]byte(input), labels.NewSymbolTable(), WithOMParserCTSeriesSkipped())
-	i := 0
+	got := testParse(t, p)
+	resetValAndLset(got) // Keep this test focused on metric, basic entries and CT only.
+	requireEntries(t, exp, got)
+}
 
-	var res labels.Labels
+func resetValAndLset(e []parsedEntry) {
-	for {
+	for i := range e {
-		et, err := p.Next()
+		e[i].v = 0
-		if errors.Is(err, io.EOF) {
+		e[i].lset = labels.EmptyLabels()
-			break
-		}
-		require.NoError(t, err)
-
-		switch et {
-		case EntrySeries:
-			p.Metric(&res)
-
-			if ct := p.CreatedTimestamp(); exp[i].isErr {
-				require.Nil(t, ct)
-			} else {
-				require.Equal(t, *exp[i].ct, *ct)
-			}
-		default:
-			i++
-			continue
-		}
-		i++
 	}
 }

model/textparse/promparse_test.go

@@ -14,33 +14,15 @@
 package textparse
 
 import (
-	"errors"
 	"io"
-	"strings"
 	"testing"
 
+	"github.com/prometheus/common/model"
 	"github.com/stretchr/testify/require"
 
-	"github.com/prometheus/common/model"
-
-	"github.com/prometheus/prometheus/model/exemplar"
 	"github.com/prometheus/prometheus/model/labels"
-	"github.com/prometheus/prometheus/util/testutil"
 )
 
-type expectedParse struct {
-	lset    labels.Labels
-	m       string
-	t       *int64
-	v       float64
-	typ     model.MetricType
-	help    string
-	unit    string
-	comment string
-	e       *exemplar.Exemplar
-	ct      *int64
-}
-
 func TestPromParse(t *testing.T) {
 	input := `# HELP go_gc_duration_seconds A summary of the GC invocation durations.
 # 	TYPE go_gc_duration_seconds summary
@@ -72,9 +54,7 @@ testmetric{label="\"bar\""} 1`
 	input += "\n# HELP metric foo\x00bar"
 	input += "\nnull_byte_metric{a=\"abc\x00\"} 1"
 
-	int64p := func(x int64) *int64 { return &x }
+	exp := []parsedEntry{
-
-	exp := []expectedParse{
 		{
 			m:    "go_gc_duration_seconds",
 			help: "A summary of the GC invocation durations.",
@@ -182,80 +162,8 @@ testmetric{label="\"bar\""} 1`
 	}
 
 	p := NewPromParser([]byte(input), labels.NewSymbolTable())
-	checkParseResults(t, p, exp)
+	got := testParse(t, p)
-}
+	requireEntries(t, exp, got)
-
-func checkParseResults(t *testing.T, p Parser, exp []expectedParse) {
-	checkParseResultsWithCT(t, p, exp, false)
-}
-
-func checkParseResultsWithCT(t *testing.T, p Parser, exp []expectedParse, ctLinesRemoved bool) {
-	i := 0
-
-	var res labels.Labels
-
-	for {
-		et, err := p.Next()
-		if errors.Is(err, io.EOF) {
-			break
-		}
-		require.NoError(t, err)
-
-		switch et {
-		case EntrySeries:
-			m, ts, v := p.Series()
-
-			p.Metric(&res)
-
-			if ctLinesRemoved {
-				// Are CT series skipped?
-				_, typ := p.Type()
-				if typeRequiresCT(typ) && strings.HasSuffix(res.Get(labels.MetricName), "_created") {
-					t.Fatalf("we exped created lines skipped")
-				}
-			}
-
-			require.Equal(t, exp[i].m, string(m))
-			require.Equal(t, exp[i].t, ts)
-			require.Equal(t, exp[i].v, v)
-			testutil.RequireEqual(t, exp[i].lset, res)
-
-			var e exemplar.Exemplar
-			found := p.Exemplar(&e)
-			if exp[i].e == nil {
-				require.False(t, found)
-			} else {
-				require.True(t, found)
-				testutil.RequireEqual(t, *exp[i].e, e)
-			}
-			if ct := p.CreatedTimestamp(); ct != nil {
-				require.Equal(t, *exp[i].ct, *ct)
-			} else {
-				require.Nil(t, exp[i].ct)
-			}
-
-		case EntryType:
-			m, typ := p.Type()
-			require.Equal(t, exp[i].m, string(m))
-			require.Equal(t, exp[i].typ, typ)
-
-		case EntryHelp:
-			m, h := p.Help()
-			require.Equal(t, exp[i].m, string(m))
-			require.Equal(t, exp[i].help, string(h))
-
-		case EntryUnit:
-			m, u := p.Unit()
-			require.Equal(t, exp[i].m, string(m))
-			require.Equal(t, exp[i].unit, string(u))
-
-		case EntryComment:
-			require.Equal(t, exp[i].comment, string(p.Comment()))
-		}
-
-		i++
-	}
-	require.Len(t, exp, i)
 }
 
 func TestUTF8PromParse(t *testing.T) {
@@ -279,7 +187,7 @@ func TestUTF8PromParse(t *testing.T) {
 {"go.gc_duration_seconds_count"} 99
 {"Heizölrückstoßabdämpfung 10€ metric with \"interesting\" {character\nchoices}","strange©™\n'quoted' \"name\""="6"} 10.0`
 
-	exp := []expectedParse{
+	exp := []parsedEntry{
 		{
 			m:    "go.gc_duration_seconds",
 			help: "A summary of the GC invocation durations.",
@@ -335,7 +243,8 @@ choices}`, "strange©™\n'quoted' \"name\"", "6"),
 	}
 
 	p := NewPromParser([]byte(input), labels.NewSymbolTable())
-	checkParseResults(t, p, exp)
+	got := testParse(t, p)
+	requireEntries(t, exp, got)
 }
 
 func TestPromParseErrors(t *testing.T) {

model/textparse/protobufparse.go

@@ -457,6 +457,12 @@ func (p *ProtobufParser) Next() (Entry, error) {
 
 		p.state = EntryHelp
 	case EntryHelp:
+		if p.mf.Unit != "" {
+			p.state = EntryUnit
+		} else {
+			p.state = EntryType
+		}
+	case EntryUnit:
 		p.state = EntryType
 	case EntryType:
 		t := p.mf.GetType()