letsencrypt
/
boulder
mirror of https://github.com/letsencrypt/boulder.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Cleanup gRPC metric formatting (#2218) 

Based on experience with the new gRPC staging deployment. gRPC generates `FullMethod` names such as `-ServiceName-MethodName` which can be confusing. For client calls to a service we actually want something formatted like `ServiceName-MethodName` and for server requests we want just `MethodName`.

This PR adds a method to clean up the `FullMethod` names returned by gRPC and formats them the way we expect.
This commit is contained in:
Roland Bracewell Shoemaker 2016-10-14 10:26:13 -07:00 committed by Jacob Hoffman-Andrews
parent 277cdf1638
commit 09483007bd
2 changed files with 54 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
grpc/interceptors.go
View File

@ -2,6 +2,7 @@ package grpc
import ( import (
"errors" "errors"
"strings"
"github.com/letsencrypt/boulder/metrics" "github.com/letsencrypt/boulder/metrics"
@ -15,13 +16,25 @@ type serverInterceptor struct {
clk clock.Clock 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) { func (si *serverInterceptor) intercept(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
if info == nil { if info == nil {
si.stats.Inc("NoInfo", 1) si.stats.Inc("NoInfo", 1)
return nil, errors.New("passed nil *grpc.UnaryServerInfo") return nil, errors.New("passed nil *grpc.UnaryServerInfo")
} }
s := si.clk.Now() s := si.clk.Now()
methodScope := si.stats.NewScope(info.FullMethod) methodScope := si.stats.NewScope(cleanMethod(info.FullMethod, true))
methodScope.Inc("Calls", 1) methodScope.Inc("Calls", 1)
methodScope.GaugeDelta("InProgress", 1) methodScope.GaugeDelta("InProgress", 1)
resp, err := handler(ctx, req) 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*. // 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 { 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() s := ci.clk.Now()
methodScope := ci.stats.NewScope(method) methodScope := ci.stats.NewScope(cleanMethod(method, false))
methodScope.Inc("Calls", 1) methodScope.Inc("Calls", 1)
methodScope.GaugeDelta("InProgress", 1) methodScope.GaugeDelta("InProgress", 1)
err := invoker(ctx, method, req, reply, cc, opts...) err := invoker(ctx, method, req, reply, cc, opts...)

58
grpc/interceptors_test.go
View File

@ -25,7 +25,7 @@ func testHandler(_ context.Context, i interface{}) (interface{}, error) {
} }
func testInvoker(_ context.Context, method string, _, _ interface{}, _ *grpc.ClientConn, _ ...grpc.CallOption) error { func testInvoker(_ context.Context, method string, _, _ interface{}, _ *grpc.ClientConn, _ ...grpc.CallOption) error {
if method == "broke-test" { if method == "-service-brokeTest" {
return errors.New("") return errors.New("")
} }
fc.Sleep(time.Second) 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().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().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) 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") 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().Inc("fake.gRPCServer.brokeTest.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().GaugeDelta("fake.gRPCServer.brokeTest.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().TimingDuration("fake.gRPCServer.brokeTest.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().GaugeDelta("fake.gRPCServer.brokeTest.InProgress", int64(-1), float32(1.0)).Return(nil)
statter.EXPECT().Inc("fake.gRPCServer.broke-test.Failed", 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: "broke-test"}, testHandler) _, 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") 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") stats := metrics.NewStatsdScope(statter, "fake", "gRPCClient")
ci := clientInterceptor{stats, fc} ci := clientInterceptor{stats, fc}
statter.EXPECT().Inc("fake.gRPCClient.test.Calls", int64(1), float32(1.0)).Return(nil) statter.EXPECT().Inc("fake.gRPCClient.service_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().GaugeDelta("fake.gRPCClient.service_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().TimingDuration("fake.gRPCClient.service_test.Latency", time.Second, float32(1.0)).Return(nil)
statter.EXPECT().GaugeDelta("fake.gRPCClient.test.InProgress", int64(-1), 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(), "test", nil, nil, nil, testInvoker) err := ci.intercept(context.Background(), "-service-test", nil, nil, nil, testInvoker)
test.AssertNotError(t, err, "ci.intercept failed with a non-nil grpc.UnaryServerInfo") 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().Inc("fake.gRPCClient.service_brokeTest.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().GaugeDelta("fake.gRPCClient.service_brokeTest.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().TimingDuration("fake.gRPCClient.service_brokeTest.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().GaugeDelta("fake.gRPCClient.service_brokeTest.InProgress", int64(-1), float32(1.0)).Return(nil)
statter.EXPECT().Inc("fake.gRPCClient.broke-test.Failed", 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(), "broke-test", nil, nil, nil, testInvoker) 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") 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)
}
}
}