Merge pull request #17305 from hxrshxz/fix-17224-remove-nhcb-check

Remove obsolete check preventing NHCB and CT zero ingestion

scrape/manager.go

@@ -184,11 +184,6 @@ func (m *Manager) reload() {
 				m.logger.Error("error reloading target set", "err", "invalid config id:"+setName)
 				continue
 			}
-			if scrapeConfig.ConvertClassicHistogramsToNHCBEnabled() && m.opts.EnableCreatedTimestampZeroIngestion {
-				// TODO(krajorama): fix https://github.com/prometheus/prometheus/issues/15137
-				m.logger.Error("error reloading target set", "err", "cannot convert classic histograms to native histograms with custom buckets and ingest created timestamp zero samples at the same time due to https://github.com/prometheus/prometheus/issues/15137")
-				continue
-			}
 			m.metrics.targetScrapePools.Inc()
 			sp, err := newScrapePool(scrapeConfig, m.append, m.offsetSeed, m.logger.With("scrape_pool", setName), m.buffers, m.opts, m.metrics)
 			if err != nil {

scrape/manager_test.go

@@ -1066,6 +1066,117 @@ func TestUnregisterMetrics(t *testing.T) {
 	}
 }
 
+// TestNHCBAndCTZeroIngestion verifies that both ConvertClassicHistogramsToNHCBEnabled
+// and EnableCreatedTimestampZeroIngestion can be used simultaneously without errors.
+// This test addresses issue #17216 by ensuring the previously blocking check has been removed.
+// The test verifies that the presence of exemplars in the input does not cause errors,
+// although exemplars are not preserved during NHCB conversion (as documented below).
+func TestNHCBAndCTZeroIngestion(t *testing.T) {
+	t.Parallel()
+
+	const (
+		mName = "test_histogram"
+		// The expected sum of the histogram, as defined by the test's OpenMetrics exposition data.
+		// This value (45.5) is the sum reported in the test_histogram_sum metric below.
+		expectedHistogramSum = 45.5
+	)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	app := &collectResultAppender{}
+	discoveryManager, scrapeManager := runManagers(t, ctx, &Options{
+		EnableCreatedTimestampZeroIngestion: true,
+		EnableNativeHistogramsIngestion:     true,
+		skipOffsetting:                      true,
+	}, &collectResultAppendable{app})
+	defer scrapeManager.Stop()
+
+	once := sync.Once{}
+	server := httptest.NewServer(
+		http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+			fail := true
+			once.Do(func() {
+				fail = false
+				w.Header().Set("Content-Type", `application/openmetrics-text`)
+
+				// Expose a histogram with created timestamp and exemplars to verify no parsing errors occur.
+				fmt.Fprint(w, `# HELP test_histogram A histogram with created timestamp and exemplars
+# TYPE test_histogram histogram
+test_histogram_bucket{le="0.0"} 1
+test_histogram_bucket{le="1.0"} 10 # {trace_id="trace-1"} 0.5 123456789
+test_histogram_bucket{le="2.0"} 20 # {trace_id="trace-2"} 1.5 123456780
+test_histogram_bucket{le="+Inf"} 30 # {trace_id="trace-3"} 2.5
+test_histogram_count 30
+test_histogram_sum 45.5
+test_histogram_created 1520430001
+# EOF
+`)
+			})
+
+			if fail {
+				w.WriteHeader(http.StatusInternalServerError)
+			}
+		}),
+	)
+	defer server.Close()
+
+	serverURL, err := url.Parse(server.URL)
+	require.NoError(t, err)
+
+	// Configuration with both convert_classic_histograms_to_nhcb enabled and CT zero ingestion enabled.
+	testConfig := fmt.Sprintf(`
+global:
+  # Use a very long scrape_interval to prevent automatic scraping during the test.
+  scrape_interval: 9999m
+  scrape_timeout: 5s
+
+scrape_configs:
+- job_name: test
+  convert_classic_histograms_to_nhcb: true
+  static_configs:
+  - targets: ['%s']
+`, serverURL.Host)
+
+	applyConfig(t, testConfig, scrapeManager, discoveryManager)
+
+	// Verify that the scrape pool was created (proves the blocking check was removed).
+	require.Eventually(t, func() bool {
+		scrapeManager.mtxScrape.Lock()
+		defer scrapeManager.mtxScrape.Unlock()
+		_, exists := scrapeManager.scrapePools["test"]
+		return exists
+	}, 5*time.Second, 100*time.Millisecond, "scrape pool should be created for job 'test'")
+
+	// Helper function to get matching histograms to avoid race conditions.
+	getMatchingHistograms := func() []histogramSample {
+		app.mtx.Lock()
+		defer app.mtx.Unlock()
+
+		var got []histogramSample
+		for _, h := range app.resultHistograms {
+			if h.metric.Get(model.MetricNameLabel) == mName {
+				got = append(got, h)
+			}
+		}
+		return got
+	}
+
+	require.Eventually(t, func() bool {
+		return len(getMatchingHistograms()) > 0
+	}, 1*time.Minute, 100*time.Millisecond, "expected histogram samples, got none")
+
+	// Verify that samples were ingested (proving both features work together).
+	got := getMatchingHistograms()
+
+	// With CT zero ingestion enabled and a created timestamp present, we expect 2 samples:
+	// one zero sample and one actual sample.
+	require.Len(t, got, 2, "expected 2 histogram samples (zero sample + actual sample)")
+	require.Equal(t, histogram.Histogram{}, *got[0].h, "first sample should be zero sample")
+	require.InDelta(t, expectedHistogramSum, got[1].h.Sum, 1e-9, "second sample should retain the expected sum")
+	require.Len(t, app.resultExemplars, 2, "expected 2 exemplars from histogram buckets")
+}
+
 func applyConfig(
 	t *testing.T,
 	config string,