allow option to convert classic histograms to nhcb entirely (don't append classic histogram series)

Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>

scrape/scrape.go

@@ -1655,10 +1655,15 @@ loop:
 					ref, err = app.AppendHistogram(ref, lset, t, nil, fh)
 				}
 			} else {
-				ref, err = app.Append(ref, lset, t, val)
+				var skipAppendFloat bool
-
 				if sl.convertClassicHistograms {
 					mName := lset.Get(labels.MetricName)
+					if !sl.scrapeClassicHistograms {
+						baseMetadata, _ := sl.cache.GetMetadata(convertnhcb.GetHistogramMetricBaseName(mName))
+						if baseMetadata.Type == model.MetricTypeHistogram {
+							skipAppendFloat = true
+						}
+					}
 					switch {
 					case strings.HasSuffix(mName, "_bucket") && lset.Has(labels.BucketLabel):
 						le, err := strconv.ParseFloat(lset.Get(labels.BucketLabel), 64)
@@ -1677,6 +1682,9 @@ loop:
 						})
 					}
 				}
+				if !skipAppendFloat {
+					ref, err = app.Append(ref, lset, t, val)
+				}
 			}
 		}
 

scrape/scrape_test.go

@@ -3380,10 +3380,14 @@ func TestConvertClassicHistograms(t *testing.T) {
 		Scheme:                   "http",
 		ScrapeInterval:           model.Duration(100 * time.Millisecond),
 		ScrapeTimeout:            model.Duration(100 * time.Millisecond),
+		ScrapeClassicHistograms:  true,
 		ConvertClassicHistograms: true,
 	}
 
 	metricsText := `
+# HELP test_metric some help text
+# TYPE test_metric counter
+test_metric 1
 # HELP test_histogram This is a histogram with default buckets
 # TYPE test_histogram histogram
 test_histogram_bucket{address="0.0.0.0",port="5001",le="0.005"} 0
@@ -3458,21 +3462,37 @@ test_histogram_count{address="0.0.0.0",port="5001"} 1
 		}
 	}
 
+	// Checks that the expected series is present and runs a basic sanity check of the values.
+	checkSeries := func(series storage.SeriesSet, encType chunkenc.ValueType) {
+		count := 0
+		for series.Next() {
+			i := series.At().Iterator(nil)
+			switch encType {
+			case chunkenc.ValFloat:
+				for i.Next() == encType {
+					_, f := i.At()
+					require.Equal(t, 1., f)
+				}
+			case chunkenc.ValHistogram:
+				for i.Next() == encType {
+					_, h := i.AtHistogram(nil)
+					require.Equal(t, uint64(1), h.Count)
+					require.Equal(t, 10.0, h.Sum)
+				}
+			}
+			count++
+		}
+		require.Equal(t, 1, count, "number of series not as expected")
+	}
+
 	series := q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram_bucket"))
 	checkValues("le", expectedLeValues, series)
 
 	series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram"))
-	count := 0
+	checkSeries(series, chunkenc.ValHistogram)
-	for series.Next() {
+
-		i := series.At().Iterator(nil)
+	series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_metric"))
-		for i.Next() == chunkenc.ValHistogram {
+	checkSeries(series, chunkenc.ValFloat)
-			_, h := i.AtHistogram(nil)
-			require.Equal(t, uint64(1), h.Count)
-			require.Equal(t, 10.0, h.Sum)
-		}
-		count++
-	}
-	require.Equal(t, 1, count, "number of series not as expected")
 }
 
 func TestScrapeLoopRunCreatesStaleMarkersOnFailedScrapeForTimestampedMetrics(t *testing.T) {

util/convertnhcb/convertnhcb.go

@@ -19,10 +19,30 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/grafana/regexp"
+
 	"github.com/prometheus/prometheus/model/histogram"
 	"github.com/prometheus/prometheus/model/labels"
 )
 
+var histogramNameSuffixReplacements = []struct {
+	pattern *regexp.Regexp
+	repl    string
+}{
+	{
+		pattern: regexp.MustCompile(`_bucket$`),
+		repl:    "",
+	},
+	{
+		pattern: regexp.MustCompile(`_sum$`),
+		repl:    "",
+	},
+	{
+		pattern: regexp.MustCompile(`_count$`),
+		repl:    "",
+	},
+}
+
 type TempHistogram struct {
 	BucketCounts map[float64]float64
 	Count        float64
@@ -143,3 +163,10 @@ func GetHistogramMetricBase(m labels.Labels, suffix string) labels.Labels {
 		Del(labels.BucketLabel).
 		Labels()
 }
+
+func GetHistogramMetricBaseName(s string) string {
+	for _, rep := range histogramNameSuffixReplacements {
+		s = rep.pattern.ReplaceAllString(s, rep.repl)
+	}
+	return s
+}