Skip to content

Commit 7b29dba

Browse files
rolandshoemakercpu
rolandshoemaker
authored and
cpu
committed
Add gRPC server-side interceptor (letsencrypt#1933)
Adds a server side unary RPC interceptor which includes basic stats. We could also use this to add a server request ID to the context.Context to identify the call through the system, but really I'd rather do that on the client side before the RPC is sent which requires the client interceptor implementation upstream. Also updates google.golang.org/grpc. Updates letsencrypt#1880.
1 parent 67fd6ef commit 7b29dba

File tree

11 files changed

+106
-16
lines changed

11 files changed

+106
-16
lines changed

‎cmd/boulder-publisher/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func main() {
4848
cmd.FailOnError(err, "Unable to create SA client")
4949

5050
if c.Publisher.GRPC != nil {
51-
s, l, err := bgrpc.NewServer(c.Publisher.GRPC)
51+
s, l, err := bgrpc.NewServer(c.Publisher.GRPC, metrics.NewStatsdScope(stats, "Publisher"))
5252
cmd.FailOnError(err, "Failed to setup gRPC server")
5353
gw := bgrpc.NewPublisherServerWrapper(pubi)
5454
pubPB.RegisterPublisherServer(s, gw)

‎cmd/boulder-va/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func main() {
9191
amqpConf := c.VA.AMQP
9292

9393
if c.VA.GRPC != nil {
94-
s, l, err := bgrpc.NewServer(c.VA.GRPC)
94+
s, l, err := bgrpc.NewServer(c.VA.GRPC, metrics.NewStatsdScope(stats, "VA"))
9595
cmd.FailOnError(err, "Unable to setup VA gRPC server")
9696
err = bgrpc.RegisterValidationAuthorityGRPCServer(s, vai)
9797
cmd.FailOnError(err, "Unable to register VA gRPC server")

‎cmd/caa-checker/server.go‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323

2424
type caaCheckerServer struct {
2525
resolver bdns.DNSResolver
26-
stats statsd.Statter
26+
stats metrics.Scope
2727
}
2828

2929
// caaSet consists of filtered CAA records
@@ -150,20 +150,20 @@ func (ccs *caaCheckerServer) checkCAA(ctx context.Context, hostname string, issu
150150

151151
if caaSet.criticalUnknown() {
152152
// Contains unknown critical directives.
153-
ccs.stats.Inc("CCS.UnknownCritical", 1, 1.0)
153+
ccs.stats.Inc("CCS.UnknownCritical", 1)
154154
return true, false, nil
155155
}
156156

157157
if len(caaSet.Unknown) > 0 {
158-
ccs.stats.Inc("CCS.WithUnknownNoncritical", 1, 1.0)
158+
ccs.stats.Inc("CCS.WithUnknownNoncritical", 1)
159159
}
160160

161161
if len(caaSet.Issue) == 0 {
162162
// Although CAA records exist, none of them pertain to issuance in this case.
163163
// (e.g. there is only an issuewild directive, but we are checking for a
164164
// non-wildcard identifier, or there is only an iodef or non-critical unknown
165165
// directive.)
166-
ccs.stats.Inc("CCS.CAA.NoneRelevant", 1, 1.0)
166+
ccs.stats.Inc("CCS.CAA.NoneRelevant", 1)
167167
return true, true, nil
168168
}
169169

@@ -174,13 +174,13 @@ func (ccs *caaCheckerServer) checkCAA(ctx context.Context, hostname string, issu
174174
// Our CAA identity must be found in the chosen checkSet.
175175
for _, caa := range caaSet.Issue {
176176
if extractIssuerDomain(caa) == issuer {
177-
ccs.stats.Inc("CCS.CAA.Authorized", 1, 1.0)
177+
ccs.stats.Inc("CCS.CAA.Authorized", 1)
178178
return true, true, nil
179179
}
180180
}
181181

182182
// The list of authorized issuers is non-empty, but we are not in it. Fail.
183-
ccs.stats.Inc("CCS.CAA.Unauthorized", 1, 1.0)
183+
ccs.stats.Inc("CCS.CAA.Unauthorized", 1)
184184
return true, false, nil
185185
}
186186

@@ -226,18 +226,19 @@ func main() {
226226

227227
stats, err := statsd.NewClient(c.StatsdServer, c.StatsdPrefix)
228228
cmd.FailOnError(err, "Failed to create StatsD client")
229+
scope := metrics.NewStatsdScope(stats, "caa-service")
229230

230231
resolver := bdns.NewDNSResolverImpl(
231232
c.DNSTimeout.Duration,
232233
[]string{c.DNSResolver},
233-
metrics.NewStatsdScope(stats, "caa-service"),
234+
scope,
234235
clock.Default(),
235236
5,
236237
)
237238

238-
s, l, err := bgrpc.NewServer(&c.GRPC)
239+
s, l, err := bgrpc.NewServer(&c.GRPC, scope)
239240
cmd.FailOnError(err, "Failed to setup gRPC server")
240-
ccs := &caaCheckerServer{resolver, stats}
241+
ccs := &caaCheckerServer{resolver, scope}
241242
pb.RegisterCAACheckerServer(s, ccs)
242243
err = s.Serve(l)
243244
cmd.FailOnError(err, "gRPC service failed")

‎cmd/caa-checker/server_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ package main
33
import (
44
"testing"
55

6-
"github.com/cactus/go-statsd-client/statsd"
76
"golang.org/x/net/context"
87

98
"github.com/letsencrypt/boulder/bdns"
109
pb "github.com/letsencrypt/boulder/cmd/caa-checker/proto"
10+
"github.com/letsencrypt/boulder/metrics"
1111
"github.com/letsencrypt/boulder/test"
1212
)
1313

@@ -42,7 +42,7 @@ func TestChecking(t *testing.T) {
4242
{"unsatisfiable.com", true, false},
4343
}
4444

45-
stats, _ := statsd.NewNoopClient()
45+
stats := metrics.NewNoopScope()
4646
ccs := &caaCheckerServer{&bdns.MockDNSResolver{}, stats}
4747
issuerDomain := "letsencrypt.org"
4848

‎grpc/interceptors.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package grpc
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
7+
"github.com/letsencrypt/boulder/metrics"
8+
9+
"github.com/jmhodges/clock"
10+
"golang.org/x/net/context"
11+
"google.golang.org/grpc"
12+
)
13+
14+
type serverInterceptor struct {
15+
stats metrics.Scope
16+
clk clock.Clock
17+
}
18+
19+
func (si *serverInterceptor) intercept(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
20+
if info == nil {
21+
si.stats.Inc("gRPC.NoInfo", 1)
22+
return nil, errors.New("passed nil *grpc.UnaryServerInfo")
23+
}
24+
si.stats.Inc(fmt.Sprintf("gRPC.%s", info.FullMethod), 1)
25+
si.stats.GaugeDelta(fmt.Sprintf("gRPC.%s.InProgress", info.FullMethod), 1)
26+
s := si.clk.Now()
27+
resp, err := handler(ctx, req)
28+
si.stats.TimingDuration(fmt.Sprintf("gRPC.%s", info.FullMethod), si.clk.Now().Sub(s))
29+
si.stats.GaugeDelta(fmt.Sprintf("gRPC.%s.InProgress", info.FullMethod), -1)
30+
if err != nil {
31+
si.stats.Inc(fmt.Sprintf("gRPC.%s.Failed", info.FullMethod), 1)
32+
}
33+
return resp, err
34+
}

‎grpc/interceptors_test.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package grpc
2+
3+
import (
4+
"errors"
5+
"testing"
6+
"time"
7+
8+
"github.com/golang/mock/gomock"
9+
"github.com/jmhodges/clock"
10+
"golang.org/x/net/context"
11+
"google.golang.org/grpc"
12+
13+
"github.com/letsencrypt/boulder/metrics"
14+
"github.com/letsencrypt/boulder/test"
15+
)
16+
17+
var fc = clock.NewFake()
18+
19+
func testHandler(_ context.Context, i interface{}) (interface{}, error) {
20+
if i != nil {
21+
return nil, errors.New("")
22+
}
23+
fc.Sleep(time.Second)
24+
return nil, nil
25+
}
26+
27+
func TestServerInterceptor(t *testing.T) {
28+
ctrl := gomock.NewController(t)
29+
defer ctrl.Finish()
30+
statter := metrics.NewMockStatter(ctrl)
31+
stats := metrics.NewStatsdScope(statter, "fake")
32+
si := serverInterceptor{stats, fc}
33+
34+
statter.EXPECT().Inc("fake.gRPC.NoInfo", int64(1), float32(1.0)).Return(nil)
35+
_, err := si.intercept(context.Background(), nil, nil, testHandler)
36+
test.AssertError(t, err, "si.intercept didn't fail with a nil grpc.UnaryServerInfo")
37+
38+
statter.EXPECT().Inc("fake.gRPC.test", int64(1), float32(1.0)).Return(nil)
39+
statter.EXPECT().GaugeDelta("fake.gRPC.test.InProgress", int64(1), float32(1.0)).Return(nil)
40+
statter.EXPECT().TimingDuration("fake.gRPC.test", time.Second, float32(1.0)).Return(nil)
41+
statter.EXPECT().GaugeDelta("fake.gRPC.test.InProgress", int64(-1), float32(1.0)).Return(nil)
42+
_, err = si.intercept(context.Background(), nil, &grpc.UnaryServerInfo{FullMethod: "test"}, testHandler)
43+
test.AssertNotError(t, err, "si.intercept failed with a non-nil grpc.UnaryServerInfo")
44+
45+
statter.EXPECT().Inc("fake.gRPC.broke-test", int64(1), float32(1.0)).Return(nil)
46+
statter.EXPECT().GaugeDelta("fake.gRPC.broke-test.InProgress", int64(1), float32(1.0)).Return(nil)
47+
statter.EXPECT().TimingDuration("fake.gRPC.broke-test", time.Duration(0), float32(1.0)).Return(nil)
48+
statter.EXPECT().GaugeDelta("fake.gRPC.broke-test.InProgress", int64(-1), float32(1.0)).Return(nil)
49+
statter.EXPECT().Inc("fake.gRPC.broke-test.Failed", int64(1), float32(1.0)).Return(nil)
50+
_, err = si.intercept(context.Background(), 0, &grpc.UnaryServerInfo{FullMethod: "broke-test"}, testHandler)
51+
test.AssertError(t, err, "si.intercept didn't fail when handler returned a error")
52+
}

‎grpc/util.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@ import (
88
"io/ioutil"
99
"net"
1010

11+
"github.com/jmhodges/clock"
1112
"google.golang.org/grpc"
1213
"google.golang.org/grpc/credentials"
1314

1415
"github.com/letsencrypt/boulder/cmd"
1516
bcreds "github.com/letsencrypt/boulder/grpc/creds"
17+
"github.com/letsencrypt/boulder/metrics"
1618
)
1719

1820
// CodedError is a alias required to appease go vet
@@ -50,7 +52,7 @@ func ClientSetup(c *cmd.GRPCClientConfig) (*grpc.ClientConn, error) {
5052
// gRPC Server that verifies the client certificate was
5153
// issued by the provided issuer certificate and presents a
5254
// a server TLS certificate.
53-
func NewServer(c *cmd.GRPCServerConfig) (*grpc.Server, net.Listener, error) {
55+
func NewServer(c *cmd.GRPCServerConfig, stats metrics.Scope) (*grpc.Server, net.Listener, error) {
5456
cert, err := tls.LoadX509KeyPair(c.ServerCertificatePath, c.ServerKeyPath)
5557
if err != nil {
5658
return nil, nil, err
@@ -73,5 +75,6 @@ func NewServer(c *cmd.GRPCServerConfig) (*grpc.Server, net.Listener, error) {
7375
if err != nil {
7476
return nil, nil, err
7577
}
76-
return grpc.NewServer(grpc.Creds(creds)), l, nil
78+
si := &serverInterceptor{stats, clock.Default()}
79+
return grpc.NewServer(grpc.Creds(creds), grpc.UnaryInterceptor(si.intercept)), l, nil
7780
}

metrics/mock_statsd_test.go renamed to metrics/mock_statsd.go

File renamed without changes.

‎metrics/scope.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//go:generate mockgen -package metrics -destination ./mock_statsd_test.go github.com/cactus/go-statsd-client/statsd Statter
1+
//go:generate mockgen -package metrics -destination ./mock_statsd.go github.com/cactus/go-statsd-client/statsd Statter
22

33
package metrics
44

‎vendor/google.golang.org/grpc/codegen.sh‎

100644100755
File mode changed.

0 commit comments

Comments
 (0)