Skip to content

Commit 020d277

Browse files
author
morvencao
committed
optimize metrics registration.
Signed-off-by: morvencao <lcao@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
1 parent efee9c1 commit 020d277

6 files changed

Lines changed: 36 additions & 34 deletions

File tree

pkg/cloudevents/server/grpc/metrics/metrics.go

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,11 @@ package metrics
33
import (
44
"context"
55
"fmt"
6-
"sync"
76
"time"
87

98
"google.golang.org/grpc"
109
"google.golang.org/grpc/status"
1110

12-
"k8s.io/component-base/metrics/legacyregistry"
13-
"k8s.io/klog/v2"
14-
1511
"github.com/cloudevents/sdk-go/v2/binding"
1612
cetypes "github.com/cloudevents/sdk-go/v2/types"
1713
pbv1 "open-cluster-management.io/sdk-go/pkg/cloudevents/generic/options/grpc/protobuf/v1"
@@ -21,9 +17,6 @@ import (
2117
k8smetrics "k8s.io/component-base/metrics"
2218
)
2319

24-
// ensure metrics are registered only once
25-
var once sync.Once
26-
2720
// subsystem used to define the metrics of grpc server for cloud events
2821
const grpcCEMetricsSubsystem = "grpc_server_ce"
2922

@@ -285,22 +278,12 @@ func SplitMethod(fullMethod string) (service, method string) {
285278
}
286279

287280
// Register all the grpc server metrics for cloudevents.
288-
func RegisterCloudEventsGRPCMetrics() {
289-
once.Do(func() {
290-
metrics := []k8smetrics.Registerable{
291-
grpcCECalledCountMetric,
292-
grpcCEProcessedCountMetric,
293-
grpcCEProcessingDurationMetric,
294-
grpcCEMessageReceivedCountMetric,
295-
grpcCEMessageSentCountMetric,
296-
}
297-
298-
for _, m := range metrics {
299-
if m != nil {
300-
legacyregistry.MustRegister(m)
301-
} else {
302-
klog.Errorf("failed to register nil grpc server metric")
303-
}
304-
}
305-
})
281+
func CloudEventsGRPCMetrics() []k8smetrics.Registerable {
282+
return []k8smetrics.Registerable{
283+
grpcCECalledCountMetric,
284+
grpcCEProcessedCountMetric,
285+
grpcCEProcessingDurationMetric,
286+
grpcCEMessageReceivedCountMetric,
287+
grpcCEMessageSentCountMetric,
288+
}
306289
}

pkg/server/grpc/metrics/metrics.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,16 @@ func NewGRPCMetricsStreamInterceptor(promServerMetrics *prom.ServerMetrics) grpc
9797
}
9898

9999
// Register all the grpc server metrics.
100-
func RegisterGRPCMetrics(promServerMetrics *prom.ServerMetrics) {
100+
func RegisterGRPCMetrics(promServerMetrics *prom.ServerMetrics, extraMetrics ...k8smetrics.Registerable) {
101101
once.Do(func() {
102102
metrics := []k8smetrics.Registerable{
103103
grpcServerConnections,
104104
grpcServerMsgRevBytes,
105105
grpcServerMsgSentBytes,
106106
}
107107

108+
metrics = append(metrics, extraMetrics...)
109+
108110
for _, m := range metrics {
109111
if m != nil {
110112
legacyregistry.MustRegister(m)

pkg/server/grpc/metrics/metrics_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,7 @@ func startBufServer(t *testing.T) *grpc.Server {
4747
grpcBroker.RegisterService(payload.ManifestBundleEventDataType, newMockWorkService())
4848
pbv1.RegisterCloudEventServiceServer(server, grpcBroker)
4949

50-
RegisterGRPCMetrics(promMiddleware)
51-
cemetrics.RegisterCloudEventsGRPCMetrics()
50+
RegisterGRPCMetrics(promMiddleware, cemetrics.CloudEventsGRPCMetrics()...)
5251
promMiddleware.InitializeMetrics(server)
5352

5453
lis = bufconn.Listen(bufSize)

pkg/server/grpc/server.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323

2424
type GRPCServer struct {
2525
options *GRPCServerOptions
26+
extraMetrics []k8smetrics.Registerable
2627
registerFuncs []func(*grpc.Server)
2728
authenticators []authn.Authenticator
2829
unaryAuthorizers []authz.UnaryAuthorizer
@@ -40,6 +41,11 @@ func (b *GRPCServer) WithRegisterFunc(registerFunc func(*grpc.Server)) *GRPCServ
4041
return b
4142
}
4243

44+
func (b *GRPCServer) WithExtraMetrics(metrics ...k8smetrics.Registerable) *GRPCServer {
45+
b.extraMetrics = append(b.extraMetrics, metrics...)
46+
return b
47+
}
48+
4349
func (b *GRPCServer) WithAuthenticator(authenticator authn.Authenticator) *GRPCServer {
4450
b.authenticators = append(b.authenticators, authenticator)
4551
return b
@@ -124,7 +130,7 @@ func (b *GRPCServer) Run(ctx context.Context) error {
124130

125131
grpcServer := grpc.NewServer(grpcServerOptions...)
126132
// register all the general grpc server metrics
127-
metrics.RegisterGRPCMetrics(promMiddleware)
133+
metrics.RegisterGRPCMetrics(promMiddleware, b.extraMetrics...)
128134
// initialize grpc server metrics with appropriate value.
129135
promMiddleware.InitializeMetrics(grpcServer)
130136

pkg/server/grpc/server_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99

1010
"google.golang.org/grpc"
1111
certutil "k8s.io/client-go/util/cert"
12+
13+
cemetrics "open-cluster-management.io/sdk-go/pkg/cloudevents/server/grpc/metrics"
1214
)
1315

1416
// testAuthenticator implements Authenticator for testing
@@ -133,6 +135,18 @@ func TestGRPCServerBuilder_WithRegisterFunc(t *testing.T) {
133135
}
134136
}
135137

138+
func TestGRPCServerBuilder_WithExtraMetrics(t *testing.T) {
139+
opt := NewGRPCServerOptions()
140+
builder := NewGRPCServer(opt)
141+
142+
// Test add extra metrics
143+
builder.WithExtraMetrics(cemetrics.CloudEventsGRPCMetrics()...)
144+
145+
if builder.extraMetrics == nil || len(builder.extraMetrics) != len(cemetrics.CloudEventsGRPCMetrics()) {
146+
t.Error("Expected extra metrics to be registered")
147+
}
148+
}
149+
136150
func TestGRPCServerBuilder_WithAuthenticator(t *testing.T) {
137151
opt := NewGRPCServerOptions()
138152
builder := NewGRPCServer(opt)

test/integration/cloudevents/suite_test.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,13 @@ import (
2525
sdkgrpc "open-cluster-management.io/sdk-go/pkg/server/grpc"
2626
grpcauthn "open-cluster-management.io/sdk-go/pkg/server/grpc/authn"
2727

28-
cemetrics "open-cluster-management.io/sdk-go/pkg/cloudevents/server/grpc/metrics"
2928
clienttesting "open-cluster-management.io/sdk-go/pkg/testing"
3029
"open-cluster-management.io/sdk-go/test/integration/cloudevents/broker/services"
3130
"open-cluster-management.io/sdk-go/test/integration/cloudevents/server"
3231
"open-cluster-management.io/sdk-go/test/integration/cloudevents/store"
3332
"open-cluster-management.io/sdk-go/test/integration/cloudevents/util"
33+
34+
cemetrics "open-cluster-management.io/sdk-go/pkg/cloudevents/server/grpc/metrics"
3435
)
3536

3637
const (
@@ -151,10 +152,7 @@ var _ = ginkgo.BeforeSuite(func(done ginkgo.Done) {
151152
WithRegisterFunc(func(s *grpc.Server) {
152153
pbv1.RegisterCloudEventServiceServer(s, grpcBroker)
153154
}).
154-
WithRegisterFunc(func(s *grpc.Server) {
155-
// register grpc metrics for cloud events
156-
cemetrics.RegisterCloudEventsGRPCMetrics()
157-
})
155+
WithExtraMetrics(cemetrics.CloudEventsGRPCMetrics()...)
158156

159157
go func() {
160158
err := grpcServer.Run(context.Background())

0 commit comments

Comments
 (0)