refactor(textparse): Introduce Variadic options in textParse.New (#17155)

* refactor(textparse): introduce ParserOptions struct for cleaner parser initialization

Signed-off-by: Naman-B-Parlecha <namanparlecha@gmail.com>

* refactor(fuzz): update fuzzParseMetricWithContentType to use ParserOptions

Signed-off-by: Naman-B-Parlecha <namanparlecha@gmail.com>

* refactor(parser): simplify ParserOptions usage in tests and implementations

Signed-off-by: Naman-B-Parlecha <namanparlecha@gmail.com>

* refactor(parse): using variadic options

Signed-off-by: Naman-B-Parlecha <namanparlecha@gmail.com>

* refactor(parser): add fallbackType & SymbolTable to variadic options

Signed-off-by: Naman-B-Parlecha <namanparlecha@gmail.com>

* refactor(parser): private fields

Signed-off-by: Naman-B-Parlecha <namanparlecha@gmail.com>

* refactor(scrape): compose parser options

Signed-off-by: Naman-B-Parlecha <namanparlecha@gmail.com>

* refactor(parser): add comments

Signed-off-by: Naman-B-Parlecha <namanparlecha@gmail.com>

* refactor(parser): update to use ParserOptions struct for configuration

Signed-off-by: Naman-B-Parlecha <namanparlecha@gmail.com>

* refactor(scrape): remove unused parserOptions field from scrapeLoop

Signed-off-by: Naman-B-Parlecha <namanparlecha@gmail.com>

* refactor(parser): update ParserOptions field names and add comments for clarity

Signed-off-by: Naman-B-Parlecha <namanparlecha@gmail.com>

---------

Signed-off-by: Naman-B-Parlecha <namanparlecha@gmail.com>

model/textparse/benchmark_test.go

@@ -157,7 +157,7 @@ func benchParse(b *testing.B, data []byte, parser string) {
 		}
 	case "omtext_with_nhcb":
 		newParserFn = func(buf []byte, st *labels.SymbolTable) Parser {
-			p, err := New(buf, "application/openmetrics-text", "", false, true, false, false, st)
+			p, err := New(buf, "application/openmetrics-text", st, ParserOptions{ConvertClassicHistogramsToNHCB: true})
 			require.NoError(b, err)
 			return p
 		}

model/textparse/interface.go

@@ -122,6 +122,28 @@ func extractMediaType(contentType, fallbackType string) (string, error) {
 	return fallbackType, fmt.Errorf("received unsupported Content-Type %q, using fallback_scrape_protocol %q", contentType, fallbackType)
 }
 
+type ParserOptions struct {
+	// EnableTypeAndUnitLabels enables parsing and inclusion of type and unit labels
+	// in the parsed metrics.
+	EnableTypeAndUnitLabels bool
+
+	// ConvertClassicHistogramsToNHCB enables conversion of classic histograms
+	// to native histogram custom buckets (NHCB) format.
+	ConvertClassicHistogramsToNHCB bool
+
+	// KeepClassicOnClassicAndNativeHistograms causes parser to output classic histogram
+	// that is also present as a native histogram. (Proto parsing only).
+	KeepClassicOnClassicAndNativeHistograms bool
+
+	// OpenMetricsSkipCTSeries determines whether to skip `_created` timestamp series
+	// during (OpenMetrics parsing only).
+	OpenMetricsSkipCTSeries bool
+
+	// FallbackContentType specifies the fallback content type to use when the provided
+	// Content-Type header cannot be parsed or is not supported.
+	FallbackContentType string
+}
+
 // New returns a new parser of the byte slice.
 //
 // This function no longer guarantees to return a valid parser.
@@ -130,27 +152,31 @@ 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, convertClassicHistogramsToNHCB, skipOMCTSeries, enableTypeAndUnitLabels bool, st *labels.SymbolTable) (Parser, error) {
+func New(b []byte, contentType string, st *labels.SymbolTable, opts ParserOptions) (Parser, error) {
-	mediaType, err := extractMediaType(contentType, fallbackType)
+	if st == nil {
+		st = labels.NewSymbolTable()
+	}
+
+	mediaType, err := extractMediaType(contentType, opts.FallbackContentType)
 	// err may be nil or something we want to warn about.
 
 	var baseParser Parser
 	switch mediaType {
 	case "application/openmetrics-text":
 		baseParser = NewOpenMetricsParser(b, st, func(o *openMetricsParserOptions) {
-			o.skipCTSeries = skipOMCTSeries
+			o.skipCTSeries = opts.OpenMetricsSkipCTSeries
-			o.enableTypeAndUnitLabels = enableTypeAndUnitLabels
+			o.enableTypeAndUnitLabels = opts.EnableTypeAndUnitLabels
 		})
 	case "application/vnd.google.protobuf":
-		return NewProtobufParser(b, parseClassicHistograms, convertClassicHistogramsToNHCB, enableTypeAndUnitLabels, st), err
+		return NewProtobufParser(b, opts.KeepClassicOnClassicAndNativeHistograms, opts.ConvertClassicHistogramsToNHCB, opts.EnableTypeAndUnitLabels, st), err
 	case "text/plain":
-		baseParser = NewPromParser(b, st, enableTypeAndUnitLabels)
+		baseParser = NewPromParser(b, st, opts.EnableTypeAndUnitLabels)
 	default:
 		return nil, err
 	}
 
-	if baseParser != nil && convertClassicHistogramsToNHCB {
+	if baseParser != nil && opts.ConvertClassicHistogramsToNHCB {
-		baseParser = NewNHCBParser(baseParser, st, parseClassicHistograms)
+		baseParser = NewNHCBParser(baseParser, st, opts.KeepClassicOnClassicAndNativeHistograms)
 	}
 
 	return baseParser, err

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, false, labels.NewSymbolTable())
+			p, err := New([]byte{}, tt.contentType, labels.NewSymbolTable(), ParserOptions{FallbackContentType: fallbackProtoMediaType})
 			tt.validateParser(t, p)
 			if tt.err == "" {
 				require.NoError(t, err)

model/textparse/nhcbparse_test.go

@@ -443,7 +443,7 @@ foobar{quantile="0.99"} 150.1`
 		},
 	}
 
-	p, err := New([]byte(input), "application/openmetrics-text", "", false, true, false, false, labels.NewSymbolTable())
+	p, err := New([]byte(input), "application/openmetrics-text", labels.NewSymbolTable(), ParserOptions{ConvertClassicHistogramsToNHCB: true})
 	require.NoError(t, err)
 	require.NotNil(t, p)
 	got := testParse(t, p)
@@ -509,7 +509,7 @@ something_bucket{a="b",le="+Inf"} 9 # {id="something-test"} 2e100 123.000
 		},
 	}
 
-	p, err := New([]byte(input), "application/openmetrics-text", "", false, true, false, false, labels.NewSymbolTable())
+	p, err := New([]byte(input), "application/openmetrics-text", labels.NewSymbolTable(), ParserOptions{ConvertClassicHistogramsToNHCB: true})
 	require.NoError(t, err)
 	require.NotNil(t, p)
 	got := testParse(t, p)
@@ -596,21 +596,21 @@ func TestNHCBParser_NoNHCBWhenExponential(t *testing.T) {
 		func() (string, parserFactory, []int, parserOptions) {
 			factory := func(keepClassic, nhcb bool) (Parser, error) {
 				inputBuf := createTestProtoBufHistogram(t)
-				return New(inputBuf.Bytes(), "application/vnd.google.protobuf", "", keepClassic, nhcb, false, false, labels.NewSymbolTable())
+				return New(inputBuf.Bytes(), "application/vnd.google.protobuf", labels.NewSymbolTable(), ParserOptions{KeepClassicOnClassicAndNativeHistograms: keepClassic, ConvertClassicHistogramsToNHCB: nhcb})
 			}
 			return "ProtoBuf", factory, []int{1, 2, 3}, parserOptions{useUTF8sep: true, hasCreatedTimeStamp: true}
 		},
 		func() (string, parserFactory, []int, parserOptions) {
 			factory := func(keepClassic, nhcb bool) (Parser, error) {
 				input := createTestOpenMetricsHistogram()
-				return New([]byte(input), "application/openmetrics-text", "", keepClassic, nhcb, false, false, labels.NewSymbolTable())
+				return New([]byte(input), "application/openmetrics-text", labels.NewSymbolTable(), ParserOptions{KeepClassicOnClassicAndNativeHistograms: keepClassic, ConvertClassicHistogramsToNHCB: nhcb})
 			}
 			return "OpenMetrics", factory, []int{1}, parserOptions{hasCreatedTimeStamp: true}
 		},
 		func() (string, parserFactory, []int, parserOptions) {
 			factory := func(keepClassic, nhcb bool) (Parser, error) {
 				input := createTestPromHistogram()
-				return New([]byte(input), "text/plain", "", keepClassic, nhcb, false, false, labels.NewSymbolTable())
+				return New([]byte(input), "text/plain", labels.NewSymbolTable(), ParserOptions{KeepClassicOnClassicAndNativeHistograms: keepClassic, ConvertClassicHistogramsToNHCB: nhcb})
 			}
 			return "Prometheus", factory, []int{1}, parserOptions{}
 		},
@@ -956,7 +956,7 @@ something_bucket{a="b",le="+Inf"} 9
 		},
 	}
 
-	p, err := New([]byte(input), "application/openmetrics-text", "", false, true, false, false, labels.NewSymbolTable())
+	p, err := New([]byte(input), "application/openmetrics-text", labels.NewSymbolTable(), ParserOptions{ConvertClassicHistogramsToNHCB: true})
 	require.NoError(t, err)
 	require.NotNil(t, p)
 	got := testParse(t, p)
@@ -1087,7 +1087,7 @@ metric: <
 		},
 	}
 
-	p, err := New(buf.Bytes(), "application/vnd.google.protobuf", "", false, true, false, false, labels.NewSymbolTable())
+	p, err := New(buf.Bytes(), "application/vnd.google.protobuf", labels.NewSymbolTable(), ParserOptions{ConvertClassicHistogramsToNHCB: true})
 	require.NoError(t, err)
 	require.NotNil(t, p)
 	got := testParse(t, p)
@@ -1149,7 +1149,7 @@ metric: <
 
 	for _, tc := range testCases {
 		t.Run(tc.typ, func(t *testing.T) {
-			p, err := New(tc.input, tc.typ, "", false, true, false, false, labels.NewSymbolTable())
+			p, err := New(tc.input, tc.typ, labels.NewSymbolTable(), ParserOptions{ConvertClassicHistogramsToNHCB: true})
 			require.NoError(t, err)
 			require.NotNil(t, p)
 

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, false, symbolTable)
+	p, warning := textparse.New(in, contentType, symbolTable, textparse.ParserOptions{})
 	if p == nil || warning != nil {
 		// An invalid content type is being passed, which should not happen
 		// in this context.

scrape/scrape.go

@@ -930,16 +930,17 @@ type scrapeLoop struct {
 	labelLimits              *labelLimits
 	interval                 time.Duration
 	timeout                  time.Duration
-	alwaysScrapeClassicHist  bool
-	convertClassicHistToNHCB bool
 	validationScheme         model.ValidationScheme
 	escapingScheme           model.EscapingScheme
+
+	alwaysScrapeClassicHist  bool
+	convertClassicHistToNHCB bool
+	enableCTZeroIngestion    bool
+	enableTypeAndUnitLabels  bool
 	fallbackScrapeProtocol   string
 
 	// Feature flagged options.
 	enableNativeHistogramIngestion bool
-	enableCTZeroIngestion          bool
-	enableTypeAndUnitLabels        bool
 
 	appender            func(ctx context.Context) storage.Appender
 	symbolTable         *labels.SymbolTable
@@ -1305,16 +1306,16 @@ func newScrapeLoop(ctx context.Context,
 		timeout:                        timeout,
 		alwaysScrapeClassicHist:        alwaysScrapeClassicHist,
 		convertClassicHistToNHCB:       convertClassicHistToNHCB,
-		enableNativeHistogramIngestion: enableNativeHistogramIngestion,
 		enableCTZeroIngestion:          enableCTZeroIngestion,
 		enableTypeAndUnitLabels:        enableTypeAndUnitLabels,
+		fallbackScrapeProtocol:         fallbackScrapeProtocol,
+		enableNativeHistogramIngestion: enableNativeHistogramIngestion,
 		reportExtraMetrics:             reportExtraMetrics,
 		appendMetadataToWAL:            appendMetadataToWAL,
 		metrics:                        metrics,
 		skipOffsetting:                 skipOffsetting,
 		validationScheme:               validationScheme,
 		escapingScheme:                 escapingScheme,
-		fallbackScrapeProtocol:         fallbackScrapeProtocol,
 	}
 	sl.ctx, sl.cancel = context.WithCancel(ctx)
 
@@ -1634,7 +1635,13 @@ func (sl *scrapeLoop) append(app storage.Appender, b []byte, contentType string,
 		return
 	}
 
-	p, err := textparse.New(b, contentType, sl.fallbackScrapeProtocol, sl.alwaysScrapeClassicHist, sl.convertClassicHistToNHCB, sl.enableCTZeroIngestion, sl.enableTypeAndUnitLabels, sl.symbolTable)
+	p, err := textparse.New(b, contentType, sl.symbolTable, textparse.ParserOptions{
+		EnableTypeAndUnitLabels:                 sl.enableTypeAndUnitLabels,
+		ConvertClassicHistogramsToNHCB:          sl.convertClassicHistToNHCB,
+		KeepClassicOnClassicAndNativeHistograms: sl.alwaysScrapeClassicHist,
+		OpenMetricsSkipCTSeries:                 sl.enableCTZeroIngestion,
+		FallbackContentType:                     sl.fallbackScrapeProtocol,
+	})
 	if p == nil {
 		sl.l.Error(
 			"Failed to determine correct type of scrape target.",

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, false, labels.NewSymbolTable())
+	p, warning := textparse.New(metric, "text/plain", labels.NewSymbolTable(), textparse.ParserOptions{})
 	require.NotNil(t, p)
 	require.NoError(t, warning)