From fb83a057457b6465c9cd0700ce31bc198bb3d19a Mon Sep 17 00:00:00 2001 From: Alexander Khristyukhin Date: Wed, 2 Feb 2022 16:58:06 +0300 Subject: [PATCH 1/4] get rid of serverHandledHistogramEnabled and associated race condition --- go.mod | 12 ++++++++++++ server_metrics.go | 45 +++++++++++++++++++++++++++++---------------- server_reporter.go | 6 +++--- 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/go.mod b/go.mod index c49d110..ddd2a27 100644 --- a/go.mod +++ b/go.mod @@ -8,3 +8,15 @@ require ( golang.org/x/net v0.0.0-20190213061140-3a22650c66bd google.golang.org/grpc v1.18.0 ) + +require ( + github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973 // indirect + github.com/davecgh/go-spew v1.1.0 // indirect + github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/prometheus/common v0.0.0-20181126121408-4724e9255275 // indirect + github.com/prometheus/procfs v0.0.0-20181204211112-1dc9a6cbc91a // indirect + golang.org/x/sys v0.0.0-20180830151530-49385e6e1522 // indirect + golang.org/x/text v0.3.0 // indirect + google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8 // indirect +) diff --git a/server_metrics.go b/server_metrics.go index d28a46e..80d5e6c 100644 --- a/server_metrics.go +++ b/server_metrics.go @@ -2,6 +2,8 @@ package grpc_prometheus import ( "context" + "sync" + "github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus" prom "github.com/prometheus/client_golang/prometheus" @@ -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()) diff --git a/server_reporter.go b/server_reporter.go index aa9db54..2681726 100644 --- a/server_reporter.go +++ b/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()) } } From ba20d16b69be278ef4b3b85e59c3961d9e9b6209 Mon Sep 17 00:00:00 2001 From: Alexander Khristyukhin Date: Wed, 2 Feb 2022 16:59:22 +0300 Subject: [PATCH 2/4] restore go.mod --- go.mod | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/go.mod b/go.mod index ddd2a27..c49d110 100644 --- a/go.mod +++ b/go.mod @@ -8,15 +8,3 @@ require ( golang.org/x/net v0.0.0-20190213061140-3a22650c66bd google.golang.org/grpc v1.18.0 ) - -require ( - github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973 // indirect - github.com/davecgh/go-spew v1.1.0 // indirect - github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect - github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/prometheus/common v0.0.0-20181126121408-4724e9255275 // indirect - github.com/prometheus/procfs v0.0.0-20181204211112-1dc9a6cbc91a // indirect - golang.org/x/sys v0.0.0-20180830151530-49385e6e1522 // indirect - golang.org/x/text v0.3.0 // indirect - google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8 // indirect -) From 86a225ce8fd18e63ef98b3c522a3c7eb2557ce69 Mon Sep 17 00:00:00 2001 From: Alexander Khristyukhin Date: Mon, 22 Aug 2022 15:24:09 +0300 Subject: [PATCH 3/4] fix go.mod --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index c49d110..92f7663 100644 --- a/go.mod +++ b/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 From 861daf151fcf34c64aa18aeabac6fd720f43fdd4 Mon Sep 17 00:00:00 2001 From: Alexander Khristyukhin Date: Tue, 23 Aug 2022 13:50:28 +0300 Subject: [PATCH 4/4] replace grpc-ecosystem --- client_test.go | 2 +- examples/grpc-server-with-prometheus/client/client.go | 4 ++-- examples/grpc-server-with-prometheus/server/server.go | 4 ++-- server_metrics.go | 2 +- server_test.go | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/client_test.go b/client_test.go index dc6e0b2..738d504 100644 --- a/client_test.go +++ b/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" diff --git a/examples/grpc-server-with-prometheus/client/client.go b/examples/grpc-server-with-prometheus/client/client.go index 14ab44b..8abd43b 100644 --- a/examples/grpc-server-with-prometheus/client/client.go +++ b/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" ) diff --git a/examples/grpc-server-with-prometheus/server/server.go b/examples/grpc-server-with-prometheus/server/server.go index 6e0d4b2..2012355 100644 --- a/examples/grpc-server-with-prometheus/server/server.go +++ b/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" ) diff --git a/server_metrics.go b/server_metrics.go index 80d5e6c..e19b117 100644 --- a/server_metrics.go +++ b/server_metrics.go @@ -4,7 +4,7 @@ import ( "context" "sync" - "github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus" + "github.com/roboslone/go-grpc-prometheus/packages/grpcstatus" prom "github.com/prometheus/client_golang/prometheus" "google.golang.org/grpc" diff --git a/server_test.go b/server_test.go index 5ee7456..bf92183 100644 --- a/server_test.go +++ b/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"