fixes after rebase

Signed-off-by: bwplotka <bwplotka@gmail.com>

scrape/manager.go

@@ -44,7 +44,7 @@ import (
 // If unsure which one to use/implement, implement AppendableV2 as it significantly simplifies implementation and allows more
 // (passing ST, always-on metadata, exemplars per sample).
 //
-// NewManager returns error if both appendable and appendableV2 is specified.
+// NewManager returns error if both appendable and appendableV2 are specified.
 //
 // Switch to AppendableV2 is in progress (https://github.com/prometheus/prometheus/issues/17632).
 // storage.Appendable will be removed soon (ETA: Q2 2026).

scrape/manager_test.go

@@ -736,7 +736,7 @@ func setupTestServer(t *testing.T, typ string, toWrite []byte) *httptest.Server
 
 // TestManagerSTZeroIngestion tests scrape manager for various ST cases.
 // NOTE(bwplotka): There is no AppenderV2 test for this STZeroIngestion feature as in V2 flow it's
-// moved to AppenderV2 implementation (e.g. storage).
+// moved to AppenderV2 implementation (e.g. storage) and it's tested there, e.g. tsdb.TestHeadAppenderV2_Append_EnableSTAsZeroSample.
 func TestManagerSTZeroIngestion(t *testing.T) {
 	t.Parallel()
 	const (
@@ -908,7 +908,7 @@ func generateTestHistogram(i int) *dto.Histogram {
 }
 
 // NOTE(bwplotka): There is no AppenderV2 test for this STZeroIngestion feature as in V2 flow it's
-// moved to AppenderV2 implementation (e.g. storage).
+// moved to AppenderV2 implementation (e.g. storage) and it's tested there, e.g. tsdb.TestHeadAppenderV2_Append_EnableSTAsZeroSample.
 func TestManagerSTZeroIngestionHistogram(t *testing.T) {
 	t.Parallel()
 	const mName = "expected_histogram"
@@ -1049,7 +1049,7 @@ func TestUnregisterMetrics(t *testing.T) {
 // although exemplars are not preserved during NHCB conversion (as documented below).
 //
 // NOTE(bwplotka): There is no AppenderV2 test for this STZeroIngestion feature as in V2 flow it's
-// moved to AppenderV2 implementation (e.g. storage).
+// moved to AppenderV2 implementation (e.g. storage) and it's tested there, e.g. tsdb.TestHeadAppenderV2_Append_EnableSTAsZeroSample.
 func TestNHCBAndSTZeroIngestion(t *testing.T) {
 	t.Parallel()
 

scrape/scrape_test.go

@@ -5771,14 +5771,10 @@ scrape_configs:
 	t.Cleanup(func() { _ = s.Close() })
 	reg := prometheus.NewRegistry()
 
-	mng, err := NewManager(&Options{DiscoveryReloadInterval: model.Duration(10 * time.Millisecond)}, nil, nil, s, nil, reg)
+	sa := selectAppendable(s, appV2)
+	mng, err := NewManager(&Options{DiscoveryReloadInterval: model.Duration(10 * time.Millisecond)}, nil, nil, sa.V1(), sa.V2(), reg)
 	require.NoError(t, err)
 
-	if appV2 {
-		mng.appendableV2 = s
-		mng.appendable = nil
-	}
-
 	cfg, err := config.Load(configStr, promslog.NewNopLogger())
 	require.NoError(t, err)
 	require.NoError(t, mng.ApplyConfig(cfg))

tsdb/head_append_v2_test.go

@@ -4111,10 +4111,18 @@ func TestHeadAppenderV2_Append_EnableSTAsZeroSample(t *testing.T) {
 	// Make sure counter resets hints are non-zero, so we can detect ST histogram samples.
 	testHistogram := tsdbutil.GenerateTestHistogram(1)
 	testHistogram.CounterResetHint = histogram.NotCounterReset
+
 	testFloatHistogram := tsdbutil.GenerateTestFloatHistogram(1)
 	testFloatHistogram.CounterResetHint = histogram.NotCounterReset
+
+	testNHCB := tsdbutil.GenerateTestCustomBucketsHistogram(1)
+	testNHCB.CounterResetHint = histogram.NotCounterReset
+
+	testFloatNHCB := tsdbutil.GenerateTestCustomBucketsFloatHistogram(1)
+	testFloatNHCB.CounterResetHint = histogram.NotCounterReset
+
 	// TODO(beorn7): Once issue #15346 is fixed, the CounterResetHint of the
-	// following two zero histograms should be histogram.CounterReset.
+	// following zero histograms should be histogram.CounterReset.
 	testZeroHistogram := &histogram.Histogram{
 		Schema:          testHistogram.Schema,
 		ZeroThreshold:   testHistogram.ZeroThreshold,
@@ -4131,6 +4139,19 @@ func TestHeadAppenderV2_Append_EnableSTAsZeroSample(t *testing.T) {
 		PositiveBuckets: []float64{0, 0, 0, 0},
 		NegativeBuckets: []float64{0, 0, 0, 0},
 	}
+	testZeroNHCB := &histogram.Histogram{
+		Schema:          testNHCB.Schema,
+		PositiveSpans:   testNHCB.PositiveSpans,
+		PositiveBuckets: []int64{0, 0, 0, 0},
+		CustomValues:    testNHCB.CustomValues,
+	}
+	testZeroFloatNHCB := &histogram.FloatHistogram{
+		Schema:          testFloatNHCB.Schema,
+		PositiveSpans:   testFloatNHCB.PositiveSpans,
+		PositiveBuckets: []float64{0, 0, 0, 0},
+		CustomValues:    testFloatNHCB.CustomValues,
+	}
+
 	type appendableSamples struct {
 		ts      int64
 		fSample float64
@@ -4183,6 +4204,34 @@ func TestHeadAppenderV2_Append_EnableSTAsZeroSample(t *testing.T) {
 				}
 			}(),
 		},
+		{
+			name: "In order ct+normal sample/NHCB",
+			appendableSamples: []appendableSamples{
+				{ts: 100, h: testNHCB, st: 1},
+				{ts: 101, h: testNHCB, st: 1},
+			},
+			expectedSamples: func() []chunks.Sample {
+				return []chunks.Sample{
+					sample{t: 1, h: testZeroNHCB},
+					sample{t: 100, h: testNHCB},
+					sample{t: 101, h: testNHCB},
+				}
+			}(),
+		},
+		{
+			name: "In order ct+normal sample/floatNHCB",
+			appendableSamples: []appendableSamples{
+				{ts: 100, fh: testFloatNHCB, st: 1},
+				{ts: 101, fh: testFloatNHCB, st: 1},
+			},
+			expectedSamples: func() []chunks.Sample {
+				return []chunks.Sample{
+					sample{t: 1, fh: testZeroFloatNHCB},
+					sample{t: 100, fh: testFloatNHCB},
+					sample{t: 101, fh: testFloatNHCB},
+				}
+			}(),
+		},
 		{
 			name: "Consecutive appends with same st ignore st/floatSample",
 			appendableSamples: []appendableSamples{
@@ -4223,6 +4272,34 @@ func TestHeadAppenderV2_Append_EnableSTAsZeroSample(t *testing.T) {
 				}
 			}(),
 		},
+		{
+			name: "Consecutive appends with same st ignore st/NHCB",
+			appendableSamples: []appendableSamples{
+				{ts: 100, h: testNHCB, st: 1},
+				{ts: 101, h: testNHCB, st: 1},
+			},
+			expectedSamples: func() []chunks.Sample {
+				return []chunks.Sample{
+					sample{t: 1, h: testZeroNHCB},
+					sample{t: 100, h: testNHCB},
+					sample{t: 101, h: testNHCB},
+				}
+			}(),
+		},
+		{
+			name: "Consecutive appends with same st ignore st/floatNHCB",
+			appendableSamples: []appendableSamples{
+				{ts: 100, fh: testFloatNHCB, st: 1},
+				{ts: 101, fh: testFloatNHCB, st: 1},
+			},
+			expectedSamples: func() []chunks.Sample {
+				return []chunks.Sample{
+					sample{t: 1, fh: testZeroFloatNHCB},
+					sample{t: 100, fh: testFloatNHCB},
+					sample{t: 101, fh: testFloatNHCB},
+				}
+			}(),
+		},
 		{
 			name: "Consecutive appends with newer st do not ignore st/floatSample",
 			appendableSamples: []appendableSamples{
@@ -4262,6 +4339,32 @@ func TestHeadAppenderV2_Append_EnableSTAsZeroSample(t *testing.T) {
 				sample{t: 102, fh: testFloatHistogram},
 			},
 		},
+		{
+			name: "Consecutive appends with newer st do not ignore st/histogram",
+			appendableSamples: []appendableSamples{
+				{ts: 100, h: testNHCB, st: 1},
+				{ts: 102, h: testNHCB, st: 101},
+			},
+			expectedSamples: []chunks.Sample{
+				sample{t: 1, h: testZeroNHCB},
+				sample{t: 100, h: testNHCB},
+				sample{t: 101, h: testZeroNHCB},
+				sample{t: 102, h: testNHCB},
+			},
+		},
+		{
+			name: "Consecutive appends with newer st do not ignore st/floathistogram",
+			appendableSamples: []appendableSamples{
+				{ts: 100, fh: testFloatNHCB, st: 1},
+				{ts: 102, fh: testFloatNHCB, st: 101},
+			},
+			expectedSamples: []chunks.Sample{
+				sample{t: 1, fh: testZeroFloatNHCB},
+				sample{t: 100, fh: testFloatNHCB},
+				sample{t: 101, fh: testZeroFloatNHCB},
+				sample{t: 102, fh: testFloatNHCB},
+			},
+		},
 		{
 			name: "ST equals to previous sample timestamp is ignored/floatSample",
 			appendableSamples: []appendableSamples{
@@ -4302,6 +4405,34 @@ func TestHeadAppenderV2_Append_EnableSTAsZeroSample(t *testing.T) {
 				}
 			}(),
 		},
+		{
+			name: "ST equals to previous sample timestamp is ignored/NHCB",
+			appendableSamples: []appendableSamples{
+				{ts: 100, h: testNHCB, st: 1},
+				{ts: 101, h: testNHCB, st: 100},
+			},
+			expectedSamples: func() []chunks.Sample {
+				return []chunks.Sample{
+					sample{t: 1, h: testZeroNHCB},
+					sample{t: 100, h: testNHCB},
+					sample{t: 101, h: testNHCB},
+				}
+			}(),
+		},
+		{
+			name: "ST equals to previous sample timestamp is ignored/floatNHCB",
+			appendableSamples: []appendableSamples{
+				{ts: 100, fh: testFloatNHCB, st: 1},
+				{ts: 101, fh: testFloatNHCB, st: 100},
+			},
+			expectedSamples: func() []chunks.Sample {
+				return []chunks.Sample{
+					sample{t: 1, fh: testZeroFloatNHCB},
+					sample{t: 100, fh: testFloatNHCB},
+					sample{t: 101, fh: testFloatNHCB},
+				}
+			}(),
+		},
 		{
 			name: "ST lower than minValidTime/float",
 			appendableSamples: []appendableSamples{
@@ -4349,6 +4480,40 @@ func TestHeadAppenderV2_Append_EnableSTAsZeroSample(t *testing.T) {
 				}
 			}(),
 		},
+		{
+			name: "ST lower than minValidTime/NHCB",
+			appendableSamples: []appendableSamples{
+				{ts: 100, h: testNHCB, st: -1},
+			},
+			// ST results ErrOutOfBounds, but ST append is best effort, so
+			// ST should be ignored, but sample appended.
+			expectedSamples: func() []chunks.Sample {
+				// NOTE: Without ST, on query, first histogram sample will get
+				// CounterReset adjusted to 0.
+				firstSample := testNHCB.Copy()
+				firstSample.CounterResetHint = histogram.UnknownCounterReset
+				return []chunks.Sample{
+					sample{t: 100, h: firstSample},
+				}
+			}(),
+		},
+		{
+			name: "ST lower than minValidTime/floatNHCB",
+			appendableSamples: []appendableSamples{
+				{ts: 100, fh: testFloatNHCB, st: -1},
+			},
+			// ST results ErrOutOfBounds, but ST append is best effort, so
+			// ST should be ignored, but sample appended.
+			expectedSamples: func() []chunks.Sample {
+				// NOTE: Without ST, on query, first histogram sample will get
+				// CounterReset adjusted to 0.
+				firstSample := testFloatNHCB.Copy()
+				firstSample.CounterResetHint = histogram.UnknownCounterReset
+				return []chunks.Sample{
+					sample{t: 100, fh: firstSample},
+				}
+			}(),
+		},
 		{
 			name: "ST duplicates an existing sample/float",
 			appendableSamples: []appendableSamples{
@@ -4402,6 +4567,44 @@ func TestHeadAppenderV2_Append_EnableSTAsZeroSample(t *testing.T) {
 				}
 			}(),
 		},
+		{
+			name: "ST duplicates an existing sample/NHCB",
+			appendableSamples: []appendableSamples{
+				{ts: 100, h: testNHCB},
+				{ts: 200, h: testNHCB, st: 100},
+			},
+			// ST results ErrDuplicateSampleForTimestamp, but ST append is best effort, so
+			// ST should be ignored, but sample appended.
+			expectedSamples: func() []chunks.Sample {
+				// NOTE: Without ST, on query, first histogram sample will get
+				// CounterReset adjusted to 0.
+				firstSample := testNHCB.Copy()
+				firstSample.CounterResetHint = histogram.UnknownCounterReset
+				return []chunks.Sample{
+					sample{t: 100, h: firstSample},
+					sample{t: 200, h: testNHCB},
+				}
+			}(),
+		},
+		{
+			name: "ST duplicates an existing sample/floatNHCB",
+			appendableSamples: []appendableSamples{
+				{ts: 100, fh: testFloatNHCB},
+				{ts: 200, fh: testFloatNHCB, st: 100},
+			},
+			// ST results ErrDuplicateSampleForTimestamp, but ST append is best effort, so
+			// ST should ignored, but sample appended.
+			expectedSamples: func() []chunks.Sample {
+				// NOTE: Without ST, on query, first histogram sample will get
+				// CounterReset adjusted to 0.
+				firstSample := testFloatNHCB.Copy()
+				firstSample.CounterResetHint = histogram.UnknownCounterReset
+				return []chunks.Sample{
+					sample{t: 100, fh: firstSample},
+					sample{t: 200, fh: testFloatNHCB},
+				}
+			}(),
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			opts := newTestHeadDefaultOptions(DefaultBlockDuration, false)