don't blindly convert series with the classic histogram name suffixes if they are not actually histograms based on metadata

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

model/textparse/nhcbparse.go

@@ -44,6 +44,9 @@ type NhcbParser struct {
 	// For Metric.
 	lset         labels.Labels
 	metricString string
+	// For Type.
+	bType []byte
+	typ   model.MetricType
 
 	// Caches the entry itself if we are inserting a converted NHCB
 	// halfway through.
@@ -96,7 +99,7 @@ func (p *NhcbParser) Help() ([]byte, []byte) {
 }
 
 func (p *NhcbParser) Type() ([]byte, model.MetricType) {
-	return p.parser.Type()
+	return p.bType, p.typ
 }
 
 func (p *NhcbParser) Unit() ([]byte, []byte) {
@@ -147,7 +150,7 @@ func (p *NhcbParser) Next() (Entry, error) {
 	case EntrySeries:
 		p.bytes, p.ts, p.value = p.parser.Series()
 		p.metricString = p.parser.Metric(&p.lset)
-		histBaseName := convertnhcb.GetHistogramMetricBaseName(p.lset)
+		histBaseName := convertnhcb.GetHistogramMetricBaseName(p.lset.Get(labels.MetricName))
 		if histBaseName == p.lastNativeHistName {
 			break
 		}
@@ -160,19 +163,17 @@ func (p *NhcbParser) Next() (Entry, error) {
 		if isNhcb := p.handleClassicHistogramSeries(p.lset); isNhcb && !p.keepClassicHistograms {
 			return p.Next()
 		}
+		return et, err
 	case EntryHistogram:
 		p.bytes, p.ts, p.h, p.fh = p.parser.Histogram()
 		p.metricString = p.parser.Metric(&p.lset)
 		p.lastNativeHistName = p.lset.Get(labels.MetricName)
-		if p.processNhcb() {
-			p.entry = et
-			return EntryHistogram, nil
-		}
-	default:
-		if p.processNhcb() {
-			p.entry = et
-			return EntryHistogram, nil
-		}
+	case EntryType:
+		p.bType, p.typ = p.parser.Type()
+	}
+	if p.processNhcb() {
+		p.entry = et
+		return EntryHistogram, nil
 	}
 	return et, err
 }
@@ -182,7 +183,13 @@ func (p *NhcbParser) Next() (Entry, error) {
 // isn't already a native histogram with the same name (assuming it is always processed
 // right before the classic histograms) and returns true if the collation was done.
 func (p *NhcbParser) handleClassicHistogramSeries(lset labels.Labels) bool {
+	if p.typ != model.MetricTypeHistogram {
+		return false
+	}
 	mName := lset.Get(labels.MetricName)
+	if convertnhcb.GetHistogramMetricBaseName(mName) != string(p.bType) {
+		return false
+	}
 	switch {
 	case strings.HasSuffix(mName, "_bucket") && lset.Has(labels.BucketLabel):
 		le, err := strconv.ParseFloat(lset.Get(labels.BucketLabel), 64)

scrape/scrape_test.go

@@ -3377,7 +3377,7 @@ func TestConvertClassicHistograms(t *testing.T) {
 			return fmt.Sprintf(`
 # HELP %s some help text
 # TYPE %s counter
-%s %d
+%s{address="0.0.0.0",port="5001"} %d
 `, name, name, name, value)
 		} else {
 			return fmt.Sprintf(`
@@ -3420,6 +3420,14 @@ name: "%s"
 help: "some help text"
 type: COUNTER
 metric: <
+	label: <
+		name: "address"
+		value: "0.0.0.0"
+	>
+	label: <
+		name: "port"
+		value: "5001"
+	>
 	counter: <
 		value: %d
 	>
@@ -3519,30 +3527,57 @@ metric: <
 		"text": {
 			text: []string{
 				genTestCounterText("test_metric_1", 1, true),
+				genTestCounterText("test_metric_1_count", 1, true),
+				genTestCounterText("test_metric_1_sum", 1, true),
+				genTestCounterText("test_metric_1_bucket", 1, true),
 				genTestHistText("test_histogram_1", true),
 				genTestCounterText("test_metric_2", 1, true),
+				genTestCounterText("test_metric_2_count", 1, true),
+				genTestCounterText("test_metric_2_sum", 1, true),
+				genTestCounterText("test_metric_2_bucket", 1, true),
 				genTestHistText("test_histogram_2", true),
 				genTestCounterText("test_metric_3", 1, true),
+				genTestCounterText("test_metric_3_count", 1, true),
+				genTestCounterText("test_metric_3_sum", 1, true),
+				genTestCounterText("test_metric_3_bucket", 1, true),
 				genTestHistText("test_histogram_3", true),
 			},
 		},
-		"text, no metadata, in different order": {
+		"text, in different order": {
 			text: []string{
-				genTestCounterText("test_metric_1", 1, false),
-				genTestHistText("test_histogram_1", false),
-				genTestCounterText("test_metric_2", 1, false),
-				genTestHistText("test_histogram_2", false),
-				genTestHistText("test_histogram_3", false),
-				genTestCounterText("test_metric_3", 1, false),
+				genTestCounterText("test_metric_1", 1, true),
+				genTestCounterText("test_metric_1_count", 1, true),
+				genTestCounterText("test_metric_1_sum", 1, true),
+				genTestCounterText("test_metric_1_bucket", 1, true),
+				genTestHistText("test_histogram_1", true),
+				genTestCounterText("test_metric_2", 1, true),
+				genTestCounterText("test_metric_2_count", 1, true),
+				genTestCounterText("test_metric_2_sum", 1, true),
+				genTestCounterText("test_metric_2_bucket", 1, true),
+				genTestHistText("test_histogram_2", true),
+				genTestHistText("test_histogram_3", true),
+				genTestCounterText("test_metric_3", 1, true),
+				genTestCounterText("test_metric_3_count", 1, true),
+				genTestCounterText("test_metric_3_sum", 1, true),
+				genTestCounterText("test_metric_3_bucket", 1, true),
 			},
 		},
 		"protobuf": {
 			text: []string{
 				genTestCounterProto("test_metric_1", 1),
+				genTestCounterProto("test_metric_1_count", 1),
+				genTestCounterProto("test_metric_1_sum", 1),
+				genTestCounterProto("test_metric_1_bucket", 1),
 				genTestHistProto("test_histogram_1", true, false),
 				genTestCounterProto("test_metric_2", 1),
+				genTestCounterProto("test_metric_2_count", 1),
+				genTestCounterProto("test_metric_2_sum", 1),
+				genTestCounterProto("test_metric_2_bucket", 1),
 				genTestHistProto("test_histogram_2", true, false),
 				genTestCounterProto("test_metric_3", 1),
+				genTestCounterProto("test_metric_3_count", 1),
+				genTestCounterProto("test_metric_3_sum", 1),
+				genTestCounterProto("test_metric_3_bucket", 1),
 				genTestHistProto("test_histogram_3", true, false),
 			},
 			contentType: "application/vnd.google.protobuf",
@@ -3551,20 +3586,38 @@ metric: <
 			text: []string{
 				genTestHistProto("test_histogram_1", true, false),
 				genTestCounterProto("test_metric_1", 1),
+				genTestCounterProto("test_metric_1_count", 1),
+				genTestCounterProto("test_metric_1_sum", 1),
+				genTestCounterProto("test_metric_1_bucket", 1),
 				genTestHistProto("test_histogram_2", true, false),
 				genTestCounterProto("test_metric_2", 1),
+				genTestCounterProto("test_metric_2_count", 1),
+				genTestCounterProto("test_metric_2_sum", 1),
+				genTestCounterProto("test_metric_2_bucket", 1),
 				genTestHistProto("test_histogram_3", true, false),
 				genTestCounterProto("test_metric_3", 1),
+				genTestCounterProto("test_metric_3_count", 1),
+				genTestCounterProto("test_metric_3_sum", 1),
+				genTestCounterProto("test_metric_3_bucket", 1),
 			},
 			contentType: "application/vnd.google.protobuf",
 		},
 		"protobuf, with native exponential histogram": {
 			text: []string{
 				genTestCounterProto("test_metric_1", 1),
+				genTestCounterProto("test_metric_1_count", 1),
+				genTestCounterProto("test_metric_1_sum", 1),
+				genTestCounterProto("test_metric_1_bucket", 1),
 				genTestHistProto("test_histogram_1", true, true),
 				genTestCounterProto("test_metric_2", 1),
+				genTestCounterProto("test_metric_2_count", 1),
+				genTestCounterProto("test_metric_2_sum", 1),
+				genTestCounterProto("test_metric_2_bucket", 1),
 				genTestHistProto("test_histogram_2", true, true),
 				genTestCounterProto("test_metric_3", 1),
+				genTestCounterProto("test_metric_3_count", 1),
+				genTestCounterProto("test_metric_3_sum", 1),
+				genTestCounterProto("test_metric_3_bucket", 1),
 				genTestHistProto("test_histogram_3", true, true),
 			},
 			contentType:    "application/vnd.google.protobuf",
@@ -3767,12 +3820,21 @@ metric: <
 					series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_metric_%d", i)))
 					checkFloatSeries(series, 1, 1.)
 
-					series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d_sum", i)))
-					checkFloatSeries(series, tc.expectedClassicHistCount, 10.)
+					series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_metric_%d_count", i)))
+					checkFloatSeries(series, 1, 1.)
+
+					series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_metric_%d_sum", i)))
+					checkFloatSeries(series, 1, 1.)
+
+					series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_metric_%d_bucket", i)))
+					checkFloatSeries(series, 1, 1.)
 
 					series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d_count", i)))
 					checkFloatSeries(series, tc.expectedClassicHistCount, 1.)
 
+					series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d_sum", i)))
+					checkFloatSeries(series, tc.expectedClassicHistCount, 10.)
+
 					series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d_bucket", i)))
 					checkBucketValues(tc.expectedClassicHistCount, metricsText.contentType, series)
 

util/convertnhcb/convertnhcb.go

@@ -164,8 +164,7 @@ func GetHistogramMetricBase(m labels.Labels, suffix string) labels.Labels {
 		Labels()
 }
 
-func GetHistogramMetricBaseName(m labels.Labels) string {
-	s := m.Get(labels.MetricName)
+func GetHistogramMetricBaseName(s string) string {
 	for _, rep := range histogramNameSuffixReplacements {
 		s = rep.pattern.ReplaceAllString(s, rep.repl)
 	}