diff --git a/model/textparse/protobufparse.go b/model/textparse/protobufparse.go index 29ccdb84d..c111bb065 100644 --- a/model/textparse/protobufparse.go +++ b/model/textparse/protobufparse.go @@ -554,20 +554,17 @@ func formatOpenMetricsFloat(f float64) string { return s + ".0" } -// isNativeHistogram returns false iff the provided histograms has no sparse -// buckets and a zero threshold of 0 and a zero count of 0. In principle, this -// could still be meant to be a native histogram (with a zero threshold of 0 and -// no observations yet), but for now, we'll treat this case as a conventional -// histogram. -// -// TODO(beorn7): In the final format, there should be an unambiguous way of -// deciding if a histogram should be ingested as a conventional one or a native -// one. +// isNativeHistogram returns false iff the provided histograms has no spans at +// all (neither positive nor negative) and a zero threshold of 0 and a zero +// count of 0. In principle, this could still be meant to be a native histogram +// with a zero threshold of 0 and no observations yet. In that case, +// instrumentation libraries should add a "no-op" span (e.g. length zero, offset +// zero) to signal that the histogram is meant to be parsed as a native +// histogram. Failing to do so will cause Prometheus to parse it as a classic +// histogram as long as no observations have happened. func isNativeHistogram(h *dto.Histogram) bool { - return h.GetZeroThreshold() > 0 || - h.GetZeroCount() > 0 || - len(h.GetNegativeDelta()) > 0 || - len(h.GetPositiveDelta()) > 0 || - len(h.GetNegativeCount()) > 0 || - len(h.GetPositiveCount()) > 0 + return len(h.GetPositiveSpan()) > 0 || + len(h.GetNegativeSpan()) > 0 || + h.GetZeroThreshold() > 0 || + h.GetZeroCount() > 0 } diff --git a/model/textparse/protobufparse_test.go b/model/textparse/protobufparse_test.go index 07f84f4ae..53523a5dd 100644 --- a/model/textparse/protobufparse_test.go +++ b/model/textparse/protobufparse_test.go @@ -517,6 +517,19 @@ metric: < sample_sum: 1.234 > > +`, + `name: "empty_histogram" +help: "A histogram without observations and with a zero threshold of zero but with a no-op span to identify it as a native histogram." +type: HISTOGRAM +metric: < + histogram: < + positive_span: < + offset: 0 + length: 0 + > + > +> + `, } @@ -965,6 +978,25 @@ func TestProtobufParse(t *testing.T) { "__name__", "without_quantiles_sum", ), }, + { + m: "empty_histogram", + help: "A histogram without observations and with a zero threshold of zero but with a no-op span to identify it as a native histogram.", + }, + { + m: "empty_histogram", + typ: MetricTypeHistogram, + }, + { + m: "empty_histogram", + shs: &histogram.Histogram{ + CounterResetHint: histogram.UnknownCounterReset, + PositiveSpans: []histogram.Span{}, + NegativeSpans: []histogram.Span{}, + }, + lset: labels.FromStrings( + "__name__", "empty_histogram", + ), + }, }, }, { @@ -1688,6 +1720,25 @@ func TestProtobufParse(t *testing.T) { "__name__", "without_quantiles_sum", ), }, + { // 78 + m: "empty_histogram", + help: "A histogram without observations and with a zero threshold of zero but with a no-op span to identify it as a native histogram.", + }, + { // 79 + m: "empty_histogram", + typ: MetricTypeHistogram, + }, + { // 80 + m: "empty_histogram", + shs: &histogram.Histogram{ + CounterResetHint: histogram.UnknownCounterReset, + PositiveSpans: []histogram.Span{}, + NegativeSpans: []histogram.Span{}, + }, + lset: labels.FromStrings( + "__name__", "empty_histogram", + ), + }, }, }, } diff --git a/prompb/io/prometheus/client/metrics.pb.go b/prompb/io/prometheus/client/metrics.pb.go index 3e4bc7df8..83a7da779 100644 --- a/prompb/io/prometheus/client/metrics.pb.go +++ b/prompb/io/prometheus/client/metrics.pb.go @@ -414,6 +414,9 @@ type Histogram struct { NegativeDelta []int64 `protobuf:"zigzag64,10,rep,packed,name=negative_delta,json=negativeDelta,proto3" json:"negative_delta,omitempty"` NegativeCount []float64 `protobuf:"fixed64,11,rep,packed,name=negative_count,json=negativeCount,proto3" json:"negative_count,omitempty"` // Positive buckets for the native histogram. + // Use a no-op span (offset 0, length 0) for a native histogram without any + // observations yet and with a zero_threshold of 0. Otherwise, it would be + // indistinguishable from a classic histogram. PositiveSpan []BucketSpan `protobuf:"bytes,12,rep,name=positive_span,json=positiveSpan,proto3" json:"positive_span"` // Use either "positive_delta" or "positive_count", the former for // regular histograms with integer counts, the latter for float diff --git a/prompb/io/prometheus/client/metrics.proto b/prompb/io/prometheus/client/metrics.proto index 6bbea622f..3fef2b6d0 100644 --- a/prompb/io/prometheus/client/metrics.proto +++ b/prompb/io/prometheus/client/metrics.proto @@ -97,6 +97,9 @@ message Histogram { repeated double negative_count = 11; // Absolute count of each bucket. // Positive buckets for the native histogram. + // Use a no-op span (offset 0, length 0) for a native histogram without any + // observations yet and with a zero_threshold of 0. Otherwise, it would be + // indistinguishable from a classic histogram. repeated BucketSpan positive_span = 12 [(gogoproto.nullable) = false]; // Use either "positive_delta" or "positive_count", the former for // regular histograms with integer counts, the latter for float