Repository: prometheus/prometheus (language: go)

Issue:
Feature Flag "Created Timestamps Zero Injection" requires enabled "Native Histograms"
### What did you do?

If you activate the feature flag `--enable-feature=created-timestamp-zero-ingestion` in Prometheus, you must also activate the feature flag `--enable-feature=native-histograms`, otherwise the following error message will appear when you scrape your own Prometheus metrics (`:9090/metrics`):

<img width="477" alt="Bildschirmfoto 2024-04-11 um 10 57 39" src="https://github.com/prometheus/prometheus/assets/20679842/148a4191-b266-4c09-ac7b-2e72ef0e7d97">

I don't know if this is intentional, but otherwise it should be documented as a dependency in the documentation?

### What did you expect to see?

_No response_

### What did you see instead? Under which circumstances?

Error: `native histograms are disabled`

### System information

_No response_

### Prometheus version

```text
prometheus, version 2.51.2 (branch: HEAD, revision: b4c0ab52c3e9b940ab803581ddae9b3d9a452337)
  build user:       root@b63f02a423d9
  build date:       20240410-14:05:54
  go version:       go1.22.2
  platform:         linux/amd64
  tags:             netgo,builtinassets,stringlabels
```


### Prometheus configuration file

_No response_

### Alertmanager version

_No response_

### Alertmanager configuration file

_No response_

### Logs

_No response_


Discussion / hints:
Thanks for the report.
Yes, IIUC, that shouldn't be required.
This looks like a regression for https://github.com/prometheus/prometheus/pull/12733 (I tested with the PR's code and the error wasn't there)

Running a binary search:

```
git bisect start 4a6f8704efcabfe9ee0f74eab58d4c11579547be 5082655392fe2eb036f24da51b54ca050496368a
git bisect run bash -c '
make common-build 
./prometheus --enable-feature=created-timestamp-zero-ingestion > /dev/null 2>&1 &
PPPID="$!"
sleep 15
last_error=$(curl -s "http://localhost:9090/api/v1/targets" | jq -er ".data.activeTargets[0].lastError")
if [ -z "$last_error" ]; then
    echo "lastError is empty."
else
    echo "lastError contains: $last_error"
    kill -9 $PPPID
    exit 13
fi
kill -9 $PPPID
'
```
returns https://github.com/prometheus/prometheus/pull/13681/commits/4d4d822c36b7fdda555aec124eb7b031ed0daa6b as the first bad commit.
But that commit only adds come native histograms metrics into prometheus'. I think https://github.com/prometheus/prometheus/pull/12733 was already broken for native histograms metrics.
Thanks for reporting! 

Yea, it seems proto parsing with native histograms assumes native histograms being enabled in the Prometheus storage, otherwise error is returned.

Skipping native histograms (dropping them), might not be a desired option either. 

I think I would be supportive on keeping this and documenting better cc @ArthurSens @beorn7 @krajorama as we want to have native histogram by default going forward, so that won't be a problem.
In that case I think `created-timestamp-zero-ingestion` should itself make all the needed changes so native histograms don't break the scraping, because when `--enable-feature=created-timestamp-zero-ingestion` is omitted, native histograms don't break the scraping when `--enable-feature=native-histograms` isn't set..

I think it's also OK to modify protobuf parsing to ignore native histograms if the feature flag is disabled.
> I think it's also OK to modify protobuf parsing to ignore native histograms if the feature flag is disabled.

Yes, just have to make sure to not ignore classic histograms as well :) 
> > I think it's also OK to modify protobuf parsing to ignore native histograms if the feature flag is disabled.
> 
> Yes, just have to make sure to not ignore classic histograms as well :)

I'm looking at the scrape.go code now... do I understand correctly that float histograms represent the classic histograms? and histogram represent native histograms? I.e, the change is simply checking if the native histogram feature flag is enabled here?

https://github.com/prometheus/prometheus/blob/f36915b6b14e3b54132a9cd701835d0255741f96/scrape/scrape.go#L1630-L1635
Both classic and native histograms have a float histogram version.
Or in other words: "classic" vs. "native" on the one hand and "integer" vs "float" on the other hand are orthogonal properties.

So no, the `if` statement you have quoted is not about switching between classic and native histograms, but about switching between integer native histograms and float native histograms.

But it _should_ be a quite easy switch.

Fix patch:
diff --git a/cmd/prometheus/main.go b/cmd/prometheus/main.go
index 0e15d5ca5fd..8218ffb18d4 100644
--- a/cmd/prometheus/main.go
+++ b/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
diff --git a/scrape/manager.go b/scrape/manager.go
index a7a8b828e59..cb92db5a8c0 100644
--- a/scrape/manager.go
+++ b/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
diff --git a/scrape/scrape.go b/scrape/scrape.go
index 4bbeab57a76..c285f05e36a 100644
--- a/scrape/scrape.go
+++ b/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,
-		buffers:                  buffers,
-		cache:                    cache,
-		appender:                 appender,
-		symbolTable:              symbolTable,
-		sampleMutator:            sampleMutator,
-		reportSampleMutator:      reportSampleMutator,
-		stopped:                  make(chan struct{}),
-		offsetSeed:               offsetSeed,
-		l:                        l,
-		parentCtx:                ctx,
-		appenderCtx:              appenderCtx,
-		honorTimestamps:          honorTimestamps,
-		trackTimestampsStaleness: trackTimestampsStaleness,
-		enableCompression:        enableCompression,
-		sampleLimit:              sampleLimit,
-		bucketLimit:              bucketLimit,
-		maxSchema:                maxSchema,
-		labelLimits:              labelLimits,
-		interval:                 interval,
-		timeout:                  timeout,
-		scrapeClassicHistograms:  scrapeClassicHistograms,
-		enableCTZeroIngestion:    enableCTZeroIngestion,
-		reportExtraMetrics:       reportExtraMetrics,
-		appendMetadataToWAL:      appendMetadataToWAL,
-		metrics:                  metrics,
-		skipOffsetting:           skipOffsetting,
+		scraper:                        sc,
+		buffers:                        buffers,
+		cache:                          cache,
+		appender:                       appender,
+		symbolTable:                    symbolTable,
+		sampleMutator:                  sampleMutator,
+		reportSampleMutator:            reportSampleMutator,
+		stopped:                        make(chan struct{}),
+		offsetSeed:                     offsetSeed,
+		l:                              l,
+		parentCtx:                      ctx,
+		appenderCtx:                    appenderCtx,
+		honorTimestamps:                honorTimestamps,
+		trackTimestampsStaleness:       trackTimestampsStaleness,
+		enableCompression:              enableCompression,
+		sampleLimit:                    sampleLimit,
+		bucketLimit:                    bucketLimit,
+		maxSchema:                      maxSchema,
+		labelLimits:                    labelLimits,
+		interval:                       interval,
+		timeout:                        timeout,
+		scrapeClassicHistograms:        scrapeClassicHistograms,
+		enableNativeHistogramIngestion: enableNativeHistogramIngestion,
+		enableCTZeroIngestion:          enableCTZeroIngestion,
+		reportExtraMetrics:             reportExtraMetrics,
+		appendMetadataToWAL:            appendMetadataToWAL,
+		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 {


Test patch:
diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go
index 20b21936b9c..51bd377e473 100644
--- a/scrape/scrape_test.go
+++ b/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
-		scrapeClassicHistograms bool
-		scrapeText              string
-		contentType             string
-		discoveryLabels         []string
-		floats                  []floatSample
-		histograms              []histogramSample
-		exemplars               []exemplar.Exemplar
+		title                           string
+		scrapeClassicHistograms         bool
+		enableNativeHistogramsIngestion bool
+		scrapeText                      string
+		contentType                     string
+		discoveryLabels                 []string
+		floats                          []floatSample
+		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)