refactor(textparse): allow for parsers with direct NHCB support (#17153)

Hide adding NHCB parser on top another parser in New() function
so we can easily add direct NHCB capable parsers.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>

model/textparse/benchmark_test.go

@@ -156,9 +156,10 @@ func benchParse(b *testing.B, data []byte, parser string) {
 			return NewOpenMetricsParser(b, st, WithOMParserCTSeriesSkipped())
 		}
 	case "omtext_with_nhcb":
-		newParserFn = func(b []byte, st *labels.SymbolTable) Parser {
+		newParserFn = func(buf []byte, st *labels.SymbolTable) Parser {
-			p := NewOpenMetricsParser(b, st, WithOMParserCTSeriesSkipped())
+			p, err := New(buf, "application/openmetrics-text", "", false, true, false, false, st)
-			return NewNHCBParser(p, st, false)
+			require.NoError(b, err)
+			return p
 		}
 	default:
 		b.Fatal("unknown parser", parser)

model/textparse/interface.go

@@ -130,23 +130,30 @@ func extractMediaType(contentType, fallbackType string) (string, error) {
 // An error may also be returned if fallbackType had to be used or there was some
 // other error parsing the supplied Content-Type.
 // If the returned parser is nil then the scrape must fail.
-func New(b []byte, contentType, fallbackType string, parseClassicHistograms, skipOMCTSeries, enableTypeAndUnitLabels bool, st *labels.SymbolTable) (Parser, error) {
+func New(b []byte, contentType, fallbackType string, parseClassicHistograms, convertClassicHistogramsToNHCB, skipOMCTSeries, enableTypeAndUnitLabels bool, st *labels.SymbolTable) (Parser, error) {
 	mediaType, err := extractMediaType(contentType, fallbackType)
 	// err may be nil or something we want to warn about.
 
+	var baseParser Parser
 	switch mediaType {
 	case "application/openmetrics-text":
-		return NewOpenMetricsParser(b, st, func(o *openMetricsParserOptions) {
+		baseParser = NewOpenMetricsParser(b, st, func(o *openMetricsParserOptions) {
 			o.skipCTSeries = skipOMCTSeries
 			o.enableTypeAndUnitLabels = enableTypeAndUnitLabels
-		}), err
+		})
 	case "application/vnd.google.protobuf":
-		return NewProtobufParser(b, parseClassicHistograms, enableTypeAndUnitLabels, st), err
+		baseParser = NewProtobufParser(b, parseClassicHistograms, enableTypeAndUnitLabels, st)
 	case "text/plain":
-		return NewPromParser(b, st, enableTypeAndUnitLabels), err
+		baseParser = NewPromParser(b, st, enableTypeAndUnitLabels)
 	default:
 		return nil, err
 	}
+
+	if baseParser != nil && convertClassicHistogramsToNHCB {
+		baseParser = NewNHCBParser(baseParser, st, parseClassicHistograms)
+	}
+
+	return baseParser, err
 }
 
 // Entry represents the type of a parsed entry.

model/textparse/interface_test.go

@@ -168,7 +168,7 @@ func TestNewParser(t *testing.T) {
 
 			fallbackProtoMediaType := tt.fallbackScrapeProtocol.HeaderMediaType()
 
-			p, err := New([]byte{}, tt.contentType, fallbackProtoMediaType, false, false, false, labels.NewSymbolTable())
+			p, err := New([]byte{}, tt.contentType, fallbackProtoMediaType, false, false, false, false, labels.NewSymbolTable())
 			tt.validateParser(t, p)
 			if tt.err == "" {
 				require.NoError(t, err)

model/textparse/nhcbparse_test.go

@@ -446,8 +446,9 @@ foobar{quantile="0.99"} 150.1`
 		},
 	}
 
-	p := NewOpenMetricsParser([]byte(input), labels.NewSymbolTable(), WithOMParserCTSeriesSkipped())
+	p, err := New([]byte(input), "application/openmetrics-text", "", false, true, false, false, labels.NewSymbolTable())
-	p = NewNHCBParser(p, labels.NewSymbolTable(), false)
+	require.NoError(t, err)
+	require.NotNil(t, p)
 	got := testParse(t, p)
 	requireEntries(t, exp, got)
 }
@@ -511,9 +512,9 @@ something_bucket{a="b",le="+Inf"} 9 # {id="something-test"} 2e100 123.000
 		},
 	}
 
-	p := NewOpenMetricsParser([]byte(input), labels.NewSymbolTable(), WithOMParserCTSeriesSkipped())
+	p, err := New([]byte(input), "application/openmetrics-text", "", false, true, false, false, labels.NewSymbolTable())
-
+	require.NoError(t, err)
-	p = NewNHCBParser(p, labels.NewSymbolTable(), false)
+	require.NotNil(t, p)
 	got := testParse(t, p)
 	requireEntries(t, exp, got)
 }
@@ -578,7 +579,7 @@ func TestNHCBParser_NoNHCBWhenExponential(t *testing.T) {
 	}
 
 	// Create parser from keep classic option.
-	type parserFactory func(bool) Parser
+	type parserFactory func(keepClassic, nhcb bool) (Parser, error)
 
 	type testCase struct {
 		name    string
@@ -596,23 +597,23 @@ func TestNHCBParser_NoNHCBWhenExponential(t *testing.T) {
 	// supported by the parser and parser options.
 	parsers := []func() (string, parserFactory, []int, parserOptions){
 		func() (string, parserFactory, []int, parserOptions) {
-			factory := func(keepClassic bool) Parser {
+			factory := func(keepClassic, nhcb bool) (Parser, error) {
 				inputBuf := createTestProtoBufHistogram(t)
-				return NewProtobufParser(inputBuf.Bytes(), keepClassic, false, labels.NewSymbolTable())
+				return New(inputBuf.Bytes(), "application/vnd.google.protobuf", "", keepClassic, nhcb, false, false, labels.NewSymbolTable())
 			}
 			return "ProtoBuf", factory, []int{1, 2, 3}, parserOptions{useUTF8sep: true, hasCreatedTimeStamp: true}
 		},
 		func() (string, parserFactory, []int, parserOptions) {
-			factory := func(bool) Parser {
+			factory := func(keepClassic, nhcb bool) (Parser, error) {
 				input := createTestOpenMetricsHistogram()
-				return NewOpenMetricsParser([]byte(input), labels.NewSymbolTable(), WithOMParserCTSeriesSkipped())
+				return New([]byte(input), "application/openmetrics-text", "", keepClassic, nhcb, false, false, labels.NewSymbolTable())
 			}
 			return "OpenMetrics", factory, []int{1}, parserOptions{hasCreatedTimeStamp: true}
 		},
 		func() (string, parserFactory, []int, parserOptions) {
-			factory := func(bool) Parser {
+			factory := func(keepClassic, nhcb bool) (Parser, error) {
 				input := createTestPromHistogram()
-				return NewPromParser([]byte(input), labels.NewSymbolTable(), false)
+				return New([]byte(input), "text/plain", "", keepClassic, nhcb, false, false, labels.NewSymbolTable())
 			}
 			return "Prometheus", factory, []int{1}, parserOptions{}
 		},
@@ -760,10 +761,9 @@ func TestNHCBParser_NoNHCBWhenExponential(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			p := tc.parser(tc.classic)
+			p, err := tc.parser(tc.classic, tc.nhcb)
-			if tc.nhcb {
+			require.NoError(t, err)
-				p = NewNHCBParser(p, labels.NewSymbolTable(), tc.classic)
+			require.NotNil(t, p)
-			}
 			got := testParse(t, p)
 			requireEntries(t, tc.exp, got)
 		})
@@ -976,8 +976,9 @@ something_bucket{a="b",le="+Inf"} 9
 		},
 	}
 
-	p := NewOpenMetricsParser([]byte(input), labels.NewSymbolTable(), WithOMParserCTSeriesSkipped())
+	p, err := New([]byte(input), "application/openmetrics-text", "", false, true, false, false, labels.NewSymbolTable())
-	p = NewNHCBParser(p, labels.NewSymbolTable(), false)
+	require.NoError(t, err)
+	require.NotNil(t, p)
 	got := testParse(t, p)
 	requireEntries(t, exp, got)
 }
@@ -1121,8 +1122,9 @@ metric: <
 		},
 	}
 
-	p := NewProtobufParser(buf.Bytes(), false, false, labels.NewSymbolTable())
+	p, err := New(buf.Bytes(), "application/vnd.google.protobuf", "", false, true, false, false, labels.NewSymbolTable())
-	p = NewNHCBParser(p, labels.NewSymbolTable(), false)
+	require.NoError(t, err)
+	require.NotNil(t, p)
 	got := testParse(t, p)
 	requireEntries(t, exp, got)
 }

promql/fuzz.go

@@ -61,7 +61,7 @@ const (
 var symbolTable = labels.NewSymbolTable()
 
 func fuzzParseMetricWithContentType(in []byte, contentType string) int {
-	p, warning := textparse.New(in, contentType, "", false, false, false, symbolTable)
+	p, warning := textparse.New(in, contentType, "", false, false, false, false, symbolTable)
 	if p == nil || warning != nil {
 		// An invalid content type is being passed, which should not happen
 		// in this context.

scrape/scrape.go

@@ -1634,7 +1634,7 @@ func (sl *scrapeLoop) append(app storage.Appender, b []byte, contentType string,
 		return
 	}
 
-	p, err := textparse.New(b, contentType, sl.fallbackScrapeProtocol, sl.alwaysScrapeClassicHist, sl.enableCTZeroIngestion, sl.enableTypeAndUnitLabels, sl.symbolTable)
+	p, err := textparse.New(b, contentType, sl.fallbackScrapeProtocol, sl.alwaysScrapeClassicHist, sl.convertClassicHistToNHCB, sl.enableCTZeroIngestion, sl.enableTypeAndUnitLabels, sl.symbolTable)
 	if p == nil {
 		sl.l.Error(
 			"Failed to determine correct type of scrape target.",
@@ -1644,9 +1644,6 @@ func (sl *scrapeLoop) append(app storage.Appender, b []byte, contentType string,
 		)
 		return
 	}
-	if sl.convertClassicHistToNHCB {
-		p = textparse.NewNHCBParser(p, sl.symbolTable, sl.alwaysScrapeClassicHist)
-	}
 	if err != nil {
 		sl.l.Debug(
 			"Invalid content type on scrape, using fallback setting.",

scrape/scrape_test.go

@@ -2097,7 +2097,7 @@ func TestScrapeLoopAppendCacheEntryButErrNotFound(t *testing.T) {
 	fakeRef := storage.SeriesRef(1)
 	expValue := float64(1)
 	metric := []byte(`metric{n="1"} 1`)
-	p, warning := textparse.New(metric, "text/plain", "", false, false, false, labels.NewSymbolTable())
+	p, warning := textparse.New(metric, "text/plain", "", false, false, false, false, labels.NewSymbolTable())
 	require.NotNil(t, p)
 	require.NoError(t, warning)