fix metadata display for summary and empty record metrics (#99)

fix #98

1. hide metadata if the metrics itself is not printed.

---------

Co-authored-by: hagen1778 <[email protected]>

Author: jiekun

metrics.go

@@ -338,6 +338,10 @@ func WriteMetadataIfNeeded(w io.Writer, metricName, metricType string) {
 		return
 	}
 	metricFamily := getMetricFamily(metricName)
+	writeMetadata(w, metricFamily, metricType)
+}
+
+func writeMetadata(w io.Writer, metricFamily, metricType string) {
 	fmt.Fprintf(w, "# HELP %s\n", metricFamily)
 	fmt.Fprintf(w, "# TYPE %s %s\n", metricFamily, metricType)
 }

set.go

@@ -37,6 +37,22 @@ func (s *Set) WritePrometheus(w io.Writer) {
 	// Collect all the metrics in in-memory buffer in order to prevent from long locking due to slow w.
 	var bb bytes.Buffer
 	lessFunc := func(i, j int) bool {
+		// the sorting must be stable.
+		// see edge cases why we can't simply do `s.a[i].name < s.a[j].name` here:
+		// https://github.com/VictoriaMetrics/metrics/pull/99#issuecomment-3277072175
+
+		// sort by metric family name first, to group the same metric family in one place.
+		fName1, fName2 := getMetricFamily(s.a[i].name), getMetricFamily(s.a[j].name)
+		if fName1 != fName2 {
+			return fName1 < fName2
+		}
+
+		// stabilize the order for summary and quantiles.
+		if s.a[i].metric.metricType() != s.a[j].metric.metricType() {
+			return s.a[i].metric.metricType() < s.a[j].metric.metricType()
+		}
+
+		// lastly by metric names, which is for quantiles and histogram buckets.
 		return s.a[i].name < s.a[j].name
 	}
 	s.mu.Lock()
@@ -50,18 +66,34 @@ func (s *Set) WritePrometheus(w io.Writer) {
 	metricsWriters := s.metricsWriters
 	s.mu.Unlock()
 
-	prevMetricFamily := ""
+	// metricsWithMetadataBuf is used to hold marshalTo temporary, and decide whether metadata is needed.
+	// it will be written to `bb` at the end and then reset for next *namedMetric in for-loop.
+	var metricsWithMetadataBuf bytes.Buffer
+	var prevMetricFamily string
 	for _, nm := range sa {
+		if !isMetadataEnabled() {
+			// Call marshalTo without the global lock, since certain metric types such as Gauge
+			// can call a callback, which, in turn, can try calling s.mu.Lock again.
+			nm.metric.marshalTo(nm.name, &bb)
+			continue
+		}
+
+		metricsWithMetadataBuf.Reset()
+		// Call marshalTo without the global lock, since certain metric types such as Gauge
+		// can call a callback, which, in turn, can try calling s.mu.Lock again.
+		nm.metric.marshalTo(nm.name, &metricsWithMetadataBuf)
+		if metricsWithMetadataBuf.Len() == 0 {
+			continue
+		}
+
 		metricFamily := getMetricFamily(nm.name)
 		if metricFamily != prevMetricFamily {
-			// write meta info only once per metric family
+			// write metadata only once per metric family
 			metricType := nm.metric.metricType()
-			WriteMetadataIfNeeded(&bb, nm.name, metricType)
+			writeMetadata(&bb, metricFamily, metricType)
 			prevMetricFamily = metricFamily
 		}
-		// Call marshalTo without the global lock, since certain metric types such as Gauge
-		// can call a callback, which, in turn, can try calling s.mu.Lock again.
-		nm.metric.marshalTo(nm.name, &bb)
+		bb.Write(metricsWithMetadataBuf.Bytes())
 	}
 	w.Write(bb.Bytes())
 

set_example_test.go

@@ -3,10 +3,13 @@ package metrics_test
 import (
 	"bytes"
 	"fmt"
+
 	"github.com/VictoriaMetrics/metrics"
 )
 
 func ExampleSet() {
+	metrics.ExposeMetadata(false)
+
 	// Create a set with a counter
 	s := metrics.NewSet()
 	sc := s.NewCounter("set_counter")
@@ -30,8 +33,7 @@ func ExampleExposeMetadata() {
 
 	s := metrics.NewSet()
 
-	sc := s.NewCounter("set_counter")
-	sc.Inc()
+	s.NewCounter("set_counter").Inc()
 
 	s.NewGauge(`unused_bytes{foo="bar"}`, func() float64 { return 58 })
 	s.NewGauge(`used_bytes{foo="bar"}`, func() float64 { return 42 })
@@ -43,6 +45,15 @@ func ExampleExposeMetadata() {
 
 	s.NewSummary("response_size_bytes").Update(1)
 
+	// response_size_bytes_extra_suffix name should verify that there are no collisions with response_size_bytes
+	s.NewSummary("response_size_bytes_extra_suffix").Update(1)
+
+	// This summary should not exist in the output, same goes to its metadata.
+	s.NewSummary(`test_summary_without_sample`)
+
+	// This summary will have quantiles printed before _sum/_count metrics
+	s.NewSummary(`vm_request_duration_seconds{path="/api/v1/query_range"}`).Update(1)
+
 	// Dump metrics from s.
 	var bb bytes.Buffer
 	s.WritePrometheus(&bb)
@@ -65,6 +76,15 @@ func ExampleExposeMetadata() {
 	// response_size_bytes{quantile="0.97"} 1
 	// response_size_bytes{quantile="0.99"} 1
 	// response_size_bytes{quantile="1"} 1
+	// # HELP response_size_bytes_extra_suffix
+	// # TYPE response_size_bytes_extra_suffix summary
+	// response_size_bytes_extra_suffix_sum 1
+	// response_size_bytes_extra_suffix_count 1
+	// response_size_bytes_extra_suffix{quantile="0.5"} 1
+	// response_size_bytes_extra_suffix{quantile="0.9"} 1
+	// response_size_bytes_extra_suffix{quantile="0.97"} 1
+	// response_size_bytes_extra_suffix{quantile="0.99"} 1
+	// response_size_bytes_extra_suffix{quantile="1"} 1
 	// # HELP set_counter
 	// # TYPE set_counter counter
 	// set_counter 1
@@ -75,4 +95,13 @@ func ExampleExposeMetadata() {
 	// # TYPE used_bytes gauge
 	// used_bytes{foo="bar"} 42
 	// used_bytes{foo="baz"} 43
+	// # HELP vm_request_duration_seconds
+	// # TYPE vm_request_duration_seconds summary
+	// vm_request_duration_seconds_sum{path="/api/v1/query_range"} 1
+	// vm_request_duration_seconds_count{path="/api/v1/query_range"} 1
+	// vm_request_duration_seconds{path="/api/v1/query_range",quantile="0.5"} 1
+	// vm_request_duration_seconds{path="/api/v1/query_range",quantile="0.9"} 1
+	// vm_request_duration_seconds{path="/api/v1/query_range",quantile="0.97"} 1
+	// vm_request_duration_seconds{path="/api/v1/query_range",quantile="0.99"} 1
+	// vm_request_duration_seconds{path="/api/v1/query_range",quantile="1"} 1
 }

summary.go

@@ -201,6 +201,10 @@ func (qv *quantileValue) marshalTo(prefix string, w io.Writer) {
 }
 
 func (qv *quantileValue) metricType() string {
+	// this metricsType should not be printed, because summary (sum and count) of the same metric family will be printed first,
+	// and if metadata is needed, the metadata from summary should be used.
+	// quantile will be printed later, so its metrics type won't be printed as metadata.
+	// See: https://github.com/VictoriaMetrics/metrics/pull/99
 	return "unsupported"
 }