bugfix: Decouple native histogram ingestions and protobuf parsing

Up until this point, if a scrape was done with the protobuf format Prometheus would always try to ingest native histograms even with the feature flag disabled. This causes problems with other feature-flags that depend on the protobuf format, like 'created-timestamp-zero-ingestion'. This commit decouples native histogram parsing from ingestion, making sure ingestion only happens when the 'native-histogram' feature-flag is enabled.

Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>

cmd/prometheus/main.go

@@ -217,6 +217,7 @@ func (c *flagConfig) setFeatureListOptions(logger log.Logger) error {
 				level.Info(logger).Log("msg", "Experimental PromQL functions enabled.")
 			case "native-histograms":
 				c.tsdb.EnableNativeHistograms = true
+				c.scrape.EnableNativeHistogramsIngestion = true
 				// Change relevant global variables. Hacky, but it's hard to pass a new option or default to unmarshallers.
 				config.DefaultConfig.GlobalConfig.ScrapeProtocols = config.DefaultProtoFirstScrapeProtocols
 				config.DefaultGlobalConfig.ScrapeProtocols = config.DefaultProtoFirstScrapeProtocols

scrape/manager.go

@@ -81,6 +81,8 @@ type Options struct {
 	// Option to enable the ingestion of the created timestamp as a synthetic zero sample.
 	// See: https://github.com/prometheus/proposals/blob/main/proposals/2023-06-13_created-timestamp.md
 	EnableCreatedTimestampZeroIngestion bool
+	// Option to enable the ingestion of native histograms.
+	EnableNativeHistogramsIngestion bool
 
 	// Optional HTTP client options to use when scraping.
 	HTTPClientOptions []config_util.HTTPClientOption

scrape/scrape.go

@@ -178,6 +178,7 @@ func newScrapePool(cfg *config.ScrapeConfig, app storage.Appendable, offsetSeed
 			opts.interval,
 			opts.timeout,
 			opts.scrapeClassicHistograms,
+			options.EnableNativeHistogramsIngestion,
 			options.EnableCreatedTimestampZeroIngestion,
 			options.ExtraMetrics,
 			options.EnableMetadataStorage,
@@ -827,7 +828,10 @@ type scrapeLoop struct {
 	interval                 time.Duration
 	timeout                  time.Duration
 	scrapeClassicHistograms  bool
-	enableCTZeroIngestion    bool
+
+	// Feature flagged options.
+	enableNativeHistogramIngestion bool
+	enableCTZeroIngestion          bool
 
 	appender            func(ctx context.Context) storage.Appender
 	symbolTable         *labels.SymbolTable
@@ -1123,6 +1127,7 @@ func newScrapeLoop(ctx context.Context,
 	interval time.Duration,
 	timeout time.Duration,
 	scrapeClassicHistograms bool,
+	enableNativeHistogramIngestion bool,
 	enableCTZeroIngestion bool,
 	reportExtraMetrics bool,
 	appendMetadataToWAL bool,
@@ -1153,33 +1158,34 @@ func newScrapeLoop(ctx context.Context,
 	}
 
 	sl := &scrapeLoop{
-		scraper:                  sc,
+		scraper:                        sc,
-		buffers:                  buffers,
+		buffers:                        buffers,
-		cache:                    cache,
+		cache:                          cache,
-		appender:                 appender,
+		appender:                       appender,
-		symbolTable:              symbolTable,
+		symbolTable:                    symbolTable,
-		sampleMutator:            sampleMutator,
+		sampleMutator:                  sampleMutator,
-		reportSampleMutator:      reportSampleMutator,
+		reportSampleMutator:            reportSampleMutator,
-		stopped:                  make(chan struct{}),
+		stopped:                        make(chan struct{}),
-		offsetSeed:               offsetSeed,
+		offsetSeed:                     offsetSeed,
-		l:                        l,
+		l:                              l,
-		parentCtx:                ctx,
+		parentCtx:                      ctx,
-		appenderCtx:              appenderCtx,
+		appenderCtx:                    appenderCtx,
-		honorTimestamps:          honorTimestamps,
+		honorTimestamps:                honorTimestamps,
-		trackTimestampsStaleness: trackTimestampsStaleness,
+		trackTimestampsStaleness:       trackTimestampsStaleness,
-		enableCompression:        enableCompression,
+		enableCompression:              enableCompression,
-		sampleLimit:              sampleLimit,
+		sampleLimit:                    sampleLimit,
-		bucketLimit:              bucketLimit,
+		bucketLimit:                    bucketLimit,
-		maxSchema:                maxSchema,
+		maxSchema:                      maxSchema,
-		labelLimits:              labelLimits,
+		labelLimits:                    labelLimits,
-		interval:                 interval,
+		interval:                       interval,
-		timeout:                  timeout,
+		timeout:                        timeout,
-		scrapeClassicHistograms:  scrapeClassicHistograms,
+		scrapeClassicHistograms:        scrapeClassicHistograms,
-		enableCTZeroIngestion:    enableCTZeroIngestion,
+		enableNativeHistogramIngestion: enableNativeHistogramIngestion,
-		reportExtraMetrics:       reportExtraMetrics,
+		enableCTZeroIngestion:          enableCTZeroIngestion,
-		appendMetadataToWAL:      appendMetadataToWAL,
+		reportExtraMetrics:             reportExtraMetrics,
-		metrics:                  metrics,
+		appendMetadataToWAL:            appendMetadataToWAL,
-		skipOffsetting:           skipOffsetting,
+		metrics:                        metrics,
+		skipOffsetting:                 skipOffsetting,
 	}
 	sl.ctx, sl.cancel = context.WithCancel(ctx)
 
@@ -1627,7 +1633,7 @@ loop:
 				}
 			}
 
-			if isHistogram {
+			if isHistogram && sl.enableNativeHistogramIngestion {
 				if h != nil {
 					ref, err = app.AppendHistogram(ref, lset, t, h, nil)
 				} else {

scrape/scrape_test.go

@@ -678,6 +678,7 @@ func newBasicScrapeLoop(t testing.TB, ctx context.Context, scraper scraper, app
 		false,
 		false,
 		false,
+		false,
 		nil,
 		false,
 		newTestScrapeMetrics(t),
@@ -819,6 +820,7 @@ func TestScrapeLoopRun(t *testing.T) {
 		false,
 		false,
 		false,
+		false,
 		nil,
 		false,
 		scrapeMetrics,
@@ -962,6 +964,7 @@ func TestScrapeLoopMetadata(t *testing.T) {
 		false,
 		false,
 		false,
+		false,
 		nil,
 		false,
 		scrapeMetrics,
@@ -1571,6 +1574,7 @@ func TestScrapeLoop_HistogramBucketLimit(t *testing.T) {
 	app := &bucketLimitAppender{Appender: resApp, limit: 2}
 
 	sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return app }, 0)
+	sl.enableNativeHistogramIngestion = true
 	sl.sampleMutator = func(l labels.Labels) labels.Labels {
 		if l.Has("deleteme") {
 			return labels.EmptyLabels()
@@ -1797,14 +1801,15 @@ func TestScrapeLoopAppendStalenessIfTrackTimestampStaleness(t *testing.T) {
 
 func TestScrapeLoopAppendExemplar(t *testing.T) {
 	tests := []struct {
-		title                   string
+		title                           string
-		scrapeClassicHistograms bool
+		scrapeClassicHistograms         bool
-		scrapeText              string
+		enableNativeHistogramsIngestion bool
-		contentType             string
+		scrapeText                      string
-		discoveryLabels         []string
+		contentType                     string
-		floats                  []floatSample
+		discoveryLabels                 []string
-		histograms              []histogramSample
+		floats                          []floatSample
-		exemplars               []exemplar.Exemplar
+		histograms                      []histogramSample
+		exemplars                       []exemplar.Exemplar
 	}{
 		{
 			title:           "Metric without exemplars",
@@ -1862,6 +1867,8 @@ metric_total{n="2"} 2 # {t="2"} 2.0 20000
 		},
 		{
 			title: "Native histogram with three exemplars",
+
+			enableNativeHistogramsIngestion: true,
 			scrapeText: `name: "test_histogram"
 help: "Test histogram with many buckets removed to keep it manageable in size."
 type: HISTOGRAM
@@ -1976,6 +1983,8 @@ metric: <
 		},
 		{
 			title: "Native histogram with three exemplars scraped as classic histogram",
+
+			enableNativeHistogramsIngestion: true,
 			scrapeText: `name: "test_histogram"
 help: "Test histogram with many buckets removed to keep it manageable in size."
 type: HISTOGRAM
@@ -2115,6 +2124,7 @@ metric: <
 			}
 
 			sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return app }, 0)
+			sl.enableNativeHistogramIngestion = test.enableNativeHistogramsIngestion
 			sl.sampleMutator = func(l labels.Labels) labels.Labels {
 				return mutateSampleLabels(l, discoveryLabels, false, nil)
 			}
@@ -3710,7 +3720,7 @@ scrape_configs:
 	s.DB.EnableNativeHistograms()
 	reg := prometheus.NewRegistry()
 
-	mng, err := NewManager(nil, nil, s, reg)
+	mng, err := NewManager(&Options{EnableNativeHistogramsIngestion: true}, nil, s, reg)
 	require.NoError(t, err)
 	cfg, err := config.Load(configStr, false, log.NewNopLogger())
 	require.NoError(t, err)