Repository: zeromicro/go-zero
Pull request #3440 (2023-07-23 17:28:19)

feat: add prometheus summary metrics
**feat**: This pr provides the code about Prometheus summary metrics

Before submitting this pr, I thought about it for a long time. In fact, Prometheus's native histogram can do what summary needs to do. However, the function of histogram only overlaps with that of suammary. In some scenarios, I don't think it can replace summary.

Here are a few reasons for adding summary metrics

+ **The histogram metric type in Prometheus also has its own issues**.

  Too low precision for calculated percentiles when the exported histogram buckets have insufficient coverage for the measurement. For example, if `http_request_duration_seconds` histogram has the following buckets: `[0-0.1]`, `(0.1-1.0]`, `(1.0-10.0]` - and the majority of requests are executed in 0.5 seconds, then all these requests will go to the `[0.1-1.0]` bucket. But it is impossible to calculate any percentile with good precision from such a data.

  Too big number of exported buckets. When users stumble upon the first issue, the most common reaction is to create big number of buckets in order to have good coverage over the measurement. This may lead to **high cardinality issues**.The high cardinality problem is something we don't want. However, there is a possibility of such problems when using histogram.

+ In many business scenarios, we often do not need to define too many quantiles. For example, in a time-consuming operation, we may only care about features of a few quantiles. However, due to the characteristics of histogram superposition, the underlying storage will be too large. This is what we don't want to see.

+ The low precision of histogram in calculating the quantile will be relatively large. In the business using go-zero, there may be a low tolerance for  low precision. At this time, maybe they hope to have a built-in summary to use.[Prometheus's comparison of histogram and summary accuracy](https://prometheus.io/docs/practices/histograms/#errors-of-quantile-estimation)

Therefore, I think it is necessary to add summary to improve the flexibility of go-zero in the selection of metrics.

At the same time, I found that there are redundant codes in the histogram options under the `/core/metric` package.

```go
HistogramVecOpts struct {
		Namespace string
		Subsystem string
		Name      string
		Help      string
		Labels    []string
		Buckets   []float64
	}
```

I think that since `VectorOpts` is defined in the metric file, we can combine it into the histogram in a more elegant way.as I defined in summary.

```go
SummaryVecOpts struct {
		VecOpt     VectorOpts
		Objectives map[float64]float64
	}
```


chenquan: Thanks for your pr, please fix the unit test.
ShyunnY: > 感谢您的公关，请修复单元测试。



> Thanks for your pr, please fix the unit test.



> 感谢您的公关，请修复单元测试。
I checked the error message of proc, and I don’t know why, when I added proc.AddShutdownListener, it always reported an error "sync: negative WaitGroup counter goroutine. I don't know what happened, I wrote according to histogram.go in terms of code logic and specification. I tried to sort the packages, but it didn't solve it. I don't know if this is a bug.


> Thanks for your pr, please fix the unit test.


ShyunnY: > Thanks for your pr, please fix the unit test.

I checked the error message of proc, and I don’t know why, when I added proc.AddShutdownListener, it always reported an error "sync: negative WaitGroup counter goroutine. I don't know what happened, I wrote according to histogram.go in terms of code logic and specification. I tried to sort the packages, but it didn't solve it. I don't know if this is a bug.
chenquan: > > Thanks for your pr, please fix the unit test.
> 
> I checked the error message of proc, and I don’t know why, when I added proc.AddShutdownListener, it always reported an error "sync: negative WaitGroup counter goroutine. I don't know what happened, I wrote according to histogram.go in terms of code logic and specification. I tried to sort the packages, but it didn't solve it. I don't know if this is a bug.

@ShyunnY 
Please merge the PR(https://github.com/ShyunnY/go-zero/pull/1) to fix the unit test
zhoushuguang: +LGTM
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/zeromicro/go-zero/pull/3440?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#3440](https://app.codecov.io/gh/zeromicro/go-zero/pull/3440?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (613b0c7) into [master](https://app.codecov.io/gh/zeromicro/go-zero/commit/42bfa26e2bae6662914a2813555fe0867e7d56fb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (42bfa26) will **increase** coverage by `0.02%`.
> The diff coverage is `90.32%`.


| [Files Changed](https://app.codecov.io/gh/zeromicro/go-zero/pull/3440?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [core/metric/summary.go](https://app.codecov.io/gh/zeromicro/go-zero/pull/3440?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29yZS9tZXRyaWMvc3VtbWFyeS5nbw==) | `89.65% <89.65%> (ø)` | |
| [core/proc/shutdown.go](https://app.codecov.io/gh/zeromicro/go-zero/pull/3440?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29yZS9wcm9jL3NodXRkb3duLmdv) | `73.91% <100.00%> (+1.18%)` | :arrow_up: |

... and [2 files with indirect coverage changes](https://app.codecov.io/gh/zeromicro/go-zero/pull/3440/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)


Patch:
diff --git a/core/metric/summary.go b/core/metric/summary.go
new file mode 100644
index 00000000..ecab0cbe
--- /dev/null
+++ b/core/metric/summary.go
@@ -0,0 +1,65 @@
+package metric
+
+import (
+	prom "github.com/prometheus/client_golang/prometheus"
+	"github.com/zeromicro/go-zero/core/proc"
+	"github.com/zeromicro/go-zero/core/prometheus"
+)
+
+type (
+	// A SummaryVecOpts is a summary vector options
+	SummaryVecOpts struct {
+		VecOpt     VectorOpts
+		Objectives map[float64]float64
+	}
+
+	// A SummaryVec interface represents a summary vector.
+	SummaryVec interface {
+		// Observe adds observation v to labels.
+		Observe(v float64, labels ...string)
+		close() bool
+	}
+
+	promSummaryVec struct {
+		summary *prom.SummaryVec
+	}
+)
+
+// NewSummaryVec return a SummaryVec
+func NewSummaryVec(cfg *SummaryVecOpts) SummaryVec {
+	if cfg == nil {
+		return nil
+	}
+
+	vec := prom.NewSummaryVec(
+		prom.SummaryOpts{
+			Namespace:  cfg.VecOpt.Namespace,
+			Subsystem:  cfg.VecOpt.Subsystem,
+			Name:       cfg.VecOpt.Name,
+			Help:       cfg.VecOpt.Help,
+			Objectives: cfg.Objectives,
+		},
+		cfg.VecOpt.Labels,
+	)
+	prom.MustRegister(vec)
+	sv := &promSummaryVec{
+		summary: vec,
+	}
+	proc.AddShutdownListener(func() {
+		sv.close()
+	})
+
+	return sv
+}
+
+func (sv *promSummaryVec) Observe(v float64, labels ...string) {
+	if !prometheus.Enabled() {
+		return
+	}
+
+	sv.summary.WithLabelValues(labels...).Observe(v)
+}
+
+func (sv *promSummaryVec) close() bool {
+	return prom.Unregister(sv.summary)
+}
diff --git a/core/proc/shutdown.go b/core/proc/shutdown.go
index 6e4e0bd5..ece547d5 100644
--- a/core/proc/shutdown.go
+++ b/core/proc/shutdown.go
@@ -96,4 +96,6 @@ func (lm *listenerManager) notifyListeners() {
 		group.RunSafe(listener)
 	}
 	group.Wait()
+
+	lm.listeners = nil
 }


Test patch:
diff --git a/core/metric/histogram_test.go b/core/metric/histogram_test.go
index 4c2f8933..4874617b 100644
--- a/core/metric/histogram_test.go
+++ b/core/metric/histogram_test.go
@@ -6,7 +6,6 @@ import (
 
 	"github.com/prometheus/client_golang/prometheus/testutil"
 	"github.com/stretchr/testify/assert"
-	"github.com/zeromicro/go-zero/core/proc"
 )
 
 func TestNewHistogramVec(t *testing.T) {
@@ -48,6 +47,4 @@ func TestHistogramObserve(t *testing.T) {
 
 	err := testutil.CollectAndCompare(hv.histogram, strings.NewReader(metadata+val))
 	assert.Nil(t, err)
-
-	proc.Shutdown()
 }
diff --git a/core/metric/summary_test.go b/core/metric/summary_test.go
new file mode 100644
index 00000000..6a599d90
--- /dev/null
+++ b/core/metric/summary_test.go
@@ -0,0 +1,68 @@
+package metric
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/prometheus/client_golang/prometheus/testutil"
+	"github.com/stretchr/testify/assert"
+	"github.com/zeromicro/go-zero/core/proc"
+)
+
+func TestNewSummaryVec(t *testing.T) {
+	summaryVec := NewSummaryVec(&SummaryVecOpts{
+		VecOpt: VectorOpts{
+			Namespace: "http_server",
+			Subsystem: "requests",
+			Name:      "duration_quantiles",
+			Help:      "rpc client requests duration(ms) φ quantiles ",
+			Labels:    []string{"method"},
+		},
+		Objectives: map[float64]float64{
+			0.5: 0.01,
+			0.9: 0.01,
+		},
+	})
+	defer summaryVec.close()
+	summaryVecNil := NewSummaryVec(nil)
+	assert.NotNil(t, summaryVec)
+	assert.Nil(t, summaryVecNil)
+}
+
+func TestSummaryObserve(t *testing.T) {
+	startAgent()
+	summaryVec := NewSummaryVec(&SummaryVecOpts{
+		VecOpt: VectorOpts{
+			Namespace: "http_server",
+			Subsystem: "requests",
+			Name:      "duration_quantiles",
+			Help:      "rpc client requests duration(ms) φ quantiles ",
+			Labels:    []string{"method"},
+		},
+		Objectives: map[float64]float64{
+			0.3: 0.01,
+			0.6: 0.01,
+			1:   0.01,
+		},
+	})
+	defer summaryVec.close()
+	sv := summaryVec.(*promSummaryVec)
+	sv.Observe(100, "GET")
+	sv.Observe(200, "GET")
+	sv.Observe(300, "GET")
+	metadata := `
+		# HELP http_server_requests_duration_quantiles rpc client requests duration(ms) φ quantiles 
+		# TYPE http_server_requests_duration_quantiles summary
+`
+	val := `
+		http_server_requests_duration_quantiles{method="GET",quantile="0.3"} 100
+		http_server_requests_duration_quantiles{method="GET",quantile="0.6"} 200
+		http_server_requests_duration_quantiles{method="GET",quantile="1"} 300
+		http_server_requests_duration_quantiles_sum{method="GET"} 600
+		http_server_requests_duration_quantiles_count{method="GET"} 3
+`
+
+	err := testutil.CollectAndCompare(sv.summary, strings.NewReader(metadata+val))
+	assert.Nil(t, err)
+	proc.Shutdown()
+}