Get rid of serverHandledHistogramEnabled and associated race condition

client_test.go

@@ -10,7 +10,7 @@ import (
 	"testing"
 	"time"
 
-	pb_testproto "github.com/grpc-ecosystem/go-grpc-prometheus/examples/testproto"
+	pb_testproto "github.com/roboslone/go-grpc-prometheus/examples/testproto"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"

examples/grpc-server-with-prometheus/client/client.go

@@ -12,8 +12,8 @@ import (
 
 	"google.golang.org/grpc"
 
-	"github.com/grpc-ecosystem/go-grpc-prometheus"
-	pb "github.com/grpc-ecosystem/go-grpc-prometheus/examples/grpc-server-with-prometheus/protobuf"
+	"github.com/roboslone/go-grpc-prometheus"
+	pb "github.com/roboslone/go-grpc-prometheus/examples/grpc-server-with-prometheus/protobuf"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 )

examples/grpc-server-with-prometheus/server/server.go

@@ -9,8 +9,8 @@ import (
 
 	"google.golang.org/grpc"
 
-	"github.com/grpc-ecosystem/go-grpc-prometheus"
-	pb "github.com/grpc-ecosystem/go-grpc-prometheus/examples/grpc-server-with-prometheus/protobuf"
+	"github.com/roboslone/go-grpc-prometheus"
+	pb "github.com/roboslone/go-grpc-prometheus/examples/grpc-server-with-prometheus/protobuf"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 )

go.mod

@@ -1,4 +1,4 @@
-module github.com/grpc-ecosystem/go-grpc-prometheus
+module github.com/roboslone/go-grpc-prometheus
 
 require (
 	github.com/golang/protobuf v1.2.0

server_metrics.go

@@ -2,7 +2,9 @@ package grpc_prometheus
 
 import (
 	"context"
-	"github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus"
+	"sync"
+
+	"github.com/roboslone/go-grpc-prometheus/packages/grpcstatus"
 	prom "github.com/prometheus/client_golang/prometheus"
 
 	"google.golang.org/grpc"
@@ -11,13 +13,13 @@ import (
 // ServerMetrics represents a collection of metrics to be registered on a
 // Prometheus metrics registry for a gRPC server.
 type ServerMetrics struct {
-	serverStartedCounter          *prom.CounterVec
-	serverHandledCounter          *prom.CounterVec
-	serverStreamMsgReceived       *prom.CounterVec
-	serverStreamMsgSent           *prom.CounterVec
-	serverHandledHistogramEnabled bool
-	serverHandledHistogramOpts    prom.HistogramOpts
-	serverHandledHistogram        *prom.HistogramVec
+	lock                       *sync.RWMutex
+	serverStartedCounter       *prom.CounterVec
+	serverHandledCounter       *prom.CounterVec
+	serverStreamMsgReceived    *prom.CounterVec
+	serverStreamMsgSent        *prom.CounterVec
+	serverHandledHistogramOpts prom.HistogramOpts
+	serverHandledHistogram     *prom.HistogramVec
 }
 
 // NewServerMetrics returns a ServerMetrics object. Use a new instance of
@@ -27,6 +29,7 @@ type ServerMetrics struct {
 func NewServerMetrics(counterOpts ...CounterOption) *ServerMetrics {
 	opts := counterOptions(counterOpts)
 	return &ServerMetrics{
+		lock: &sync.RWMutex{},
 		serverStartedCounter: prom.NewCounterVec(
 			opts.apply(prom.CounterOpts{
 				Name: "grpc_server_started_total",
@@ -47,7 +50,6 @@ func NewServerMetrics(counterOpts ...CounterOption) *ServerMetrics {
 				Name: "grpc_server_msg_sent_total",
 				Help: "Total number of gRPC stream messages sent by the server.",
 			}), []string{"grpc_type", "grpc_service", "grpc_method"}),
-		serverHandledHistogramEnabled: false,
 		serverHandledHistogramOpts: prom.HistogramOpts{
 			Name:    "grpc_server_handling_seconds",
 			Help:    "Histogram of response latency (seconds) of gRPC that had been application-level handled by the server.",
@@ -62,16 +64,24 @@ func NewServerMetrics(counterOpts ...CounterOption) *ServerMetrics {
 // expensive on Prometheus servers. It takes options to configure histogram
 // options such as the defined buckets.
 func (m *ServerMetrics) EnableHandlingTimeHistogram(opts ...HistogramOption) {
+	m.lock.Lock()
+	defer m.lock.Unlock()
+
 	for _, o := range opts {
 		o(&m.serverHandledHistogramOpts)
 	}
-	if !m.serverHandledHistogramEnabled {
+	if m.serverHandledHistogram == nil {
 		m.serverHandledHistogram = prom.NewHistogramVec(
 			m.serverHandledHistogramOpts,
 			[]string{"grpc_type", "grpc_service", "grpc_method"},
 		)
 	}
-	m.serverHandledHistogramEnabled = true
+}
+
+func (m *ServerMetrics) getServerHandledHistogram() *prom.HistogramVec {
+	m.lock.RLock()
+	defer m.lock.RUnlock()
+	return m.serverHandledHistogram
 }
 
 // Describe sends the super-set of all possible descriptors of metrics
@@ -82,8 +92,9 @@ func (m *ServerMetrics) Describe(ch chan<- *prom.Desc) {
 	m.serverHandledCounter.Describe(ch)
 	m.serverStreamMsgReceived.Describe(ch)
 	m.serverStreamMsgSent.Describe(ch)
-	if m.serverHandledHistogramEnabled {
-		m.serverHandledHistogram.Describe(ch)
+
+	if h := m.getServerHandledHistogram(); h != nil {
+		h.Describe(ch)
 	}
 }
 
@@ -95,8 +106,9 @@ func (m *ServerMetrics) Collect(ch chan<- prom.Metric) {
 	m.serverHandledCounter.Collect(ch)
 	m.serverStreamMsgReceived.Collect(ch)
 	m.serverStreamMsgSent.Collect(ch)
-	if m.serverHandledHistogramEnabled {
-		m.serverHandledHistogram.Collect(ch)
+
+	if h := m.getServerHandledHistogram(); h != nil {
+		h.Collect(ch)
 	}
 }
 
@@ -177,8 +189,9 @@ func preRegisterMethod(metrics *ServerMetrics, serviceName string, mInfo *grpc.M
 	metrics.serverStartedCounter.GetMetricWithLabelValues(methodType, serviceName, methodName)
 	metrics.serverStreamMsgReceived.GetMetricWithLabelValues(methodType, serviceName, methodName)
 	metrics.serverStreamMsgSent.GetMetricWithLabelValues(methodType, serviceName, methodName)
-	if metrics.serverHandledHistogramEnabled {
-		metrics.serverHandledHistogram.GetMetricWithLabelValues(methodType, serviceName, methodName)
+
+	if h := metrics.getServerHandledHistogram(); h != nil {
+		h.GetMetricWithLabelValues(methodType, serviceName, methodName)
 	}
 	for _, code := range allCodes {
 		metrics.serverHandledCounter.GetMetricWithLabelValues(methodType, serviceName, methodName, code.String())

server_reporter.go

@@ -22,7 +22,7 @@ func newServerReporter(m *ServerMetrics, rpcType grpcType, fullMethod string) *s
 		metrics: m,
 		rpcType: rpcType,
 	}
-	if r.metrics.serverHandledHistogramEnabled {
+	if r.metrics.getServerHandledHistogram() != nil {
 		r.startTime = time.Now()
 	}
 	r.serviceName, r.methodName = splitMethodName(fullMethod)
@@ -40,7 +40,7 @@ func (r *serverReporter) SentMessage() {
 
 func (r *serverReporter) Handled(code codes.Code) {
 	r.metrics.serverHandledCounter.WithLabelValues(string(r.rpcType), r.serviceName, r.methodName, code.String()).Inc()
-	if r.metrics.serverHandledHistogramEnabled {
-		r.metrics.serverHandledHistogram.WithLabelValues(string(r.rpcType), r.serviceName, r.methodName).Observe(time.Since(r.startTime).Seconds())
+	if h := r.metrics.getServerHandledHistogram(); h != nil {
+		h.WithLabelValues(string(r.rpcType), r.serviceName, r.methodName).Observe(time.Since(r.startTime).Seconds())
 	}
 }

server_test.go

@@ -19,7 +19,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"github.com/prometheus/client_golang/prometheus/testutil"
 
-	pb_testproto "github.com/grpc-ecosystem/go-grpc-prometheus/examples/testproto"
+	pb_testproto "github.com/roboslone/go-grpc-prometheus/examples/testproto"
 	"github.com/prometheus/client_golang/prometheus"
 	dto "github.com/prometheus/client_model/go"
 	"github.com/stretchr/testify/assert"