diff --git a/grpc/interceptors.go b/grpc/interceptors.go index 37f1cae69..16d4d9e6a 100644 --- a/grpc/interceptors.go +++ b/grpc/interceptors.go @@ -2,6 +2,7 @@ package grpc import ( "errors" + "strings" "github.com/letsencrypt/boulder/metrics" @@ -15,13 +16,25 @@ type serverInterceptor struct { clk clock.Clock } +func cleanMethod(m string, trimService bool) string { + m = strings.TrimLeft(m, "-") + if trimService { + s := strings.Split(m, "-") + if len(s) == 1 { + return m + } + return s[len(s)-1] + } + return strings.Replace(m, "-", "_", -1) +} + func (si *serverInterceptor) intercept(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { if info == nil { si.stats.Inc("NoInfo", 1) return nil, errors.New("passed nil *grpc.UnaryServerInfo") } s := si.clk.Now() - methodScope := si.stats.NewScope(info.FullMethod) + methodScope := si.stats.NewScope(cleanMethod(info.FullMethod, true)) methodScope.Inc("Calls", 1) methodScope.GaugeDelta("InProgress", 1) resp, err := handler(ctx, req) @@ -42,7 +55,7 @@ type clientInterceptor struct { // is currently experimental the metrics it reports should be kept as stable as can be, *within reason*. func (ci *clientInterceptor) intercept(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { s := ci.clk.Now() - methodScope := ci.stats.NewScope(method) + methodScope := ci.stats.NewScope(cleanMethod(method, false)) methodScope.Inc("Calls", 1) methodScope.GaugeDelta("InProgress", 1) err := invoker(ctx, method, req, reply, cc, opts...) diff --git a/grpc/interceptors_test.go b/grpc/interceptors_test.go index 12396f571..0140cb952 100644 --- a/grpc/interceptors_test.go +++ b/grpc/interceptors_test.go @@ -25,7 +25,7 @@ func testHandler(_ context.Context, i interface{}) (interface{}, error) { } func testInvoker(_ context.Context, method string, _, _ interface{}, _ *grpc.ClientConn, _ ...grpc.CallOption) error { - if method == "broke-test" { + if method == "-service-brokeTest" { return errors.New("") } fc.Sleep(time.Second) @@ -47,15 +47,15 @@ func TestServerInterceptor(t *testing.T) { statter.EXPECT().GaugeDelta("fake.gRPCServer.test.InProgress", int64(1), float32(1.0)).Return(nil) statter.EXPECT().TimingDuration("fake.gRPCServer.test.Latency", time.Second, float32(1.0)).Return(nil) statter.EXPECT().GaugeDelta("fake.gRPCServer.test.InProgress", int64(-1), float32(1.0)).Return(nil) - _, err = si.intercept(context.Background(), nil, &grpc.UnaryServerInfo{FullMethod: "test"}, testHandler) + _, err = si.intercept(context.Background(), nil, &grpc.UnaryServerInfo{FullMethod: "-service-test"}, testHandler) test.AssertNotError(t, err, "si.intercept failed with a non-nil grpc.UnaryServerInfo") - statter.EXPECT().Inc("fake.gRPCServer.broke-test.Calls", int64(1), float32(1.0)).Return(nil) - statter.EXPECT().GaugeDelta("fake.gRPCServer.broke-test.InProgress", int64(1), float32(1.0)).Return(nil) - statter.EXPECT().TimingDuration("fake.gRPCServer.broke-test.Latency", time.Duration(0), float32(1.0)).Return(nil) - statter.EXPECT().GaugeDelta("fake.gRPCServer.broke-test.InProgress", int64(-1), float32(1.0)).Return(nil) - statter.EXPECT().Inc("fake.gRPCServer.broke-test.Failed", int64(1), float32(1.0)).Return(nil) - _, err = si.intercept(context.Background(), 0, &grpc.UnaryServerInfo{FullMethod: "broke-test"}, testHandler) + statter.EXPECT().Inc("fake.gRPCServer.brokeTest.Calls", int64(1), float32(1.0)).Return(nil) + statter.EXPECT().GaugeDelta("fake.gRPCServer.brokeTest.InProgress", int64(1), float32(1.0)).Return(nil) + statter.EXPECT().TimingDuration("fake.gRPCServer.brokeTest.Latency", time.Duration(0), float32(1.0)).Return(nil) + statter.EXPECT().GaugeDelta("fake.gRPCServer.brokeTest.InProgress", int64(-1), float32(1.0)).Return(nil) + statter.EXPECT().Inc("fake.gRPCServer.brokeTest.Failed", int64(1), float32(1.0)).Return(nil) + _, err = si.intercept(context.Background(), 0, &grpc.UnaryServerInfo{FullMethod: "brokeTest"}, testHandler) test.AssertError(t, err, "si.intercept didn't fail when handler returned a error") } @@ -66,18 +66,38 @@ func TestClientInterceptor(t *testing.T) { stats := metrics.NewStatsdScope(statter, "fake", "gRPCClient") ci := clientInterceptor{stats, fc} - statter.EXPECT().Inc("fake.gRPCClient.test.Calls", int64(1), float32(1.0)).Return(nil) - statter.EXPECT().GaugeDelta("fake.gRPCClient.test.InProgress", int64(1), float32(1.0)).Return(nil) - statter.EXPECT().TimingDuration("fake.gRPCClient.test.Latency", time.Second, float32(1.0)).Return(nil) - statter.EXPECT().GaugeDelta("fake.gRPCClient.test.InProgress", int64(-1), float32(1.0)).Return(nil) - err := ci.intercept(context.Background(), "test", nil, nil, nil, testInvoker) + statter.EXPECT().Inc("fake.gRPCClient.service_test.Calls", int64(1), float32(1.0)).Return(nil) + statter.EXPECT().GaugeDelta("fake.gRPCClient.service_test.InProgress", int64(1), float32(1.0)).Return(nil) + statter.EXPECT().TimingDuration("fake.gRPCClient.service_test.Latency", time.Second, float32(1.0)).Return(nil) + statter.EXPECT().GaugeDelta("fake.gRPCClient.service_test.InProgress", int64(-1), float32(1.0)).Return(nil) + err := ci.intercept(context.Background(), "-service-test", nil, nil, nil, testInvoker) test.AssertNotError(t, err, "ci.intercept failed with a non-nil grpc.UnaryServerInfo") - statter.EXPECT().Inc("fake.gRPCClient.broke-test.Calls", int64(1), float32(1.0)).Return(nil) - statter.EXPECT().GaugeDelta("fake.gRPCClient.broke-test.InProgress", int64(1), float32(1.0)).Return(nil) - statter.EXPECT().TimingDuration("fake.gRPCClient.broke-test.Latency", time.Duration(0), float32(1.0)).Return(nil) - statter.EXPECT().GaugeDelta("fake.gRPCClient.broke-test.InProgress", int64(-1), float32(1.0)).Return(nil) - statter.EXPECT().Inc("fake.gRPCClient.broke-test.Failed", int64(1), float32(1.0)).Return(nil) - err = ci.intercept(context.Background(), "broke-test", nil, nil, nil, testInvoker) + statter.EXPECT().Inc("fake.gRPCClient.service_brokeTest.Calls", int64(1), float32(1.0)).Return(nil) + statter.EXPECT().GaugeDelta("fake.gRPCClient.service_brokeTest.InProgress", int64(1), float32(1.0)).Return(nil) + statter.EXPECT().TimingDuration("fake.gRPCClient.service_brokeTest.Latency", time.Duration(0), float32(1.0)).Return(nil) + statter.EXPECT().GaugeDelta("fake.gRPCClient.service_brokeTest.InProgress", int64(-1), float32(1.0)).Return(nil) + statter.EXPECT().Inc("fake.gRPCClient.service_brokeTest.Failed", int64(1), float32(1.0)).Return(nil) + err = ci.intercept(context.Background(), "-service-brokeTest", nil, nil, nil, testInvoker) test.AssertError(t, err, "ci.intercept didn't fail when handler returned a error") } + +func TestCleanMethod(t *testing.T) { + tests := []struct { + in string + out string + stripService bool + }{ + {"-ServiceName-MethodName", "ServiceName_MethodName", false}, + {"-ServiceName-MethodName", "MethodName", true}, + {"--MethodName", "MethodName", true}, + {"--MethodName", "MethodName", true}, + {"MethodName", "MethodName", false}, + } + for _, tc := range tests { + out := cleanMethod(tc.in, tc.stripService) + if out != tc.out { + t.Fatalf("cleanMethod didn't return the expected name: expected: %q, got: %q", tc.out, out) + } + } +}