Skip to content

Commit

Permalink
refactor(admin): replace RPC logging interceptor (#508)
Browse files Browse the repository at this point in the history
### What this PR does

This change replaces the RPC logging interceptor of admin from `github.com/grpc-ecosystem/go-grpc-middleware` with our simple interceptor. It makes the interceptor simple and lightweight.
  • Loading branch information
ijsong authored Jul 28, 2023
2 parents d82177a + 5f2ec49 commit e9b46ad
Show file tree
Hide file tree
Showing 35 changed files with 63 additions and 2,212 deletions.
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ require (
github.com/gogo/status v1.1.1
github.com/golang/mock v1.6.0
github.com/golang/protobuf v1.5.3
github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
github.com/lib/pq v1.10.9
github.com/pkg/errors v0.9.1
github.com/puzpuzpuz/xsync/v2 v2.4.1
Expand Down
7 changes: 0 additions & 7 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,6 @@ github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORR
github.com/gopherjs/gopherjs v1.17.2 h1:fQnZVsXk8uxXIStYb0N4bGk7jeyTalG/wsZjQ25dO0g=
github.com/gopherjs/gopherjs v1.17.2/go.mod h1:pRRIvn/QzFLrKfvEz3qUuEhtE/zLCWfreZ6J5gM2i+k=
github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 h1:+9834+KizmvFV7pXQGSXQTsaWhq2GjuNUt0aUU0YBYw=
github.com/grpc-ecosystem/go-grpc-middleware v1.3.0/go.mod h1:z0ButlSOZa5vEBq9m2m2hlwIgKw+rp3sdCBRoJY+30Y=
github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw=
github.com/grpc-ecosystem/grpc-gateway/v2 v2.7.0 h1:BZHcxBETFHIdVyhyEfOvn/RdU/QGdLI4y34qQGjGWO0=
github.com/grpc-ecosystem/grpc-gateway/v2 v2.7.0/go.mod h1:hgWBS7lorOAVIJEQMi4ZsPv9hVvWI6+ch50m39Pf2Ks=
Expand Down Expand Up @@ -361,7 +359,6 @@ github.com/onsi/ginkgo/v2 v2.9.1 h1:zie5Ly042PD3bsCvsSOPvRnFwyo3rKe64TJlD6nu0mk=
github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY=
github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
github.com/onsi/gomega v1.27.4 h1:Z2AnStgsdSayCMDiCU42qIz+HLqEPcgiOCXjAU/w+8E=
github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o=
github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
github.com/pingcap/errors v0.11.4 h1:lFuQV/oaUMGcD2tqt+01ROSmJs75VG1ToEOkZIZ4nE4=
github.com/pingcap/errors v0.11.4/go.mod h1:Oi8TUi2kEtXXLMJk9l1cGmz20kV3TaQ0usTwv5KuLY8=
Expand Down Expand Up @@ -519,17 +516,14 @@ go.opentelemetry.io/otel/trace v1.16.0/go.mod h1:Yt9vYq1SdNz3xdjZZK7wcXv1qv2pwLk
go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI=
go.opentelemetry.io/proto/otlp v0.19.0 h1:IVN6GR+mhC4s5yfcTbmzHYODqvWAp3ZedA2SJPI1Nnw=
go.opentelemetry.io/proto/otlp v0.19.0/go.mod h1:H7XAot3MsfNsj7EXtrA2q5xSNQ10UqI405h3+duxN4U=
go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw=
go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
go.uber.org/automaxprocs v1.5.3 h1:kWazyxZUrS3Gs4qUpbwo5kEIMGe/DAvi5Z4tl2NW4j8=
go.uber.org/automaxprocs v1.5.3/go.mod h1:eRbA25aqJrxAbsLO0xy5jVwPt7FQnRgjW+efnwa1WM0=
go.uber.org/goleak v1.2.1 h1:NBol2c7O1ZokfZ0LEU9K6Whx/KnwvepVetCUhtKja4A=
go.uber.org/goleak v1.2.1/go.mod h1:qlT2yGI9QafXHhZZLxlSuNsMw3FFLxBr+tBRlmO1xH4=
go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0=
go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0=
go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y=
go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q=
go.uber.org/zap v1.24.0 h1:FiJd5l1UOLj0wCgbSE0rwwXHzEdAZS6hiiSnxJN/D60=
go.uber.org/zap v1.24.0/go.mod h1:2kMP+WWQ8aoFoedH3T2sq6iJ2yDWpHbP0f6MQbS9Gkg=
golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
Expand Down Expand Up @@ -808,7 +802,6 @@ google.golang.org/genproto v0.0.0-20200228133532-8c2c7df3a383/go.mod h1:55QSHmfG
google.golang.org/genproto v0.0.0-20200305110556-506484158171/go.mod h1:55QSHmfGQM9UVYDPBsyGGes0y52j32PQ3BqQfXhyH3c=
google.golang.org/genproto v0.0.0-20200312145019-da6875a35672/go.mod h1:55QSHmfGQM9UVYDPBsyGGes0y52j32PQ3BqQfXhyH3c=
google.golang.org/genproto v0.0.0-20200331122359-1ee6d9798940/go.mod h1:55QSHmfGQM9UVYDPBsyGGes0y52j32PQ3BqQfXhyH3c=
google.golang.org/genproto v0.0.0-20200423170343-7949de9c1215/go.mod h1:55QSHmfGQM9UVYDPBsyGGes0y52j32PQ3BqQfXhyH3c=
google.golang.org/genproto v0.0.0-20200430143042-b979b6f78d84/go.mod h1:55QSHmfGQM9UVYDPBsyGGes0y52j32PQ3BqQfXhyH3c=
google.golang.org/genproto v0.0.0-20200511104702-f5ebc3bea380/go.mod h1:55QSHmfGQM9UVYDPBsyGGes0y52j32PQ3BqQfXhyH3c=
google.golang.org/genproto v0.0.0-20200513103714-09dca8ec2884/go.mod h1:55QSHmfGQM9UVYDPBsyGGes0y52j32PQ3BqQfXhyH3c=
Expand Down
13 changes: 3 additions & 10 deletions internal/admin/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ import (

"github.com/gogo/protobuf/proto"
"github.com/gogo/status"
grpcmiddleware "github.com/grpc-ecosystem/go-grpc-middleware"
grpczap "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap"
grpcctxtags "github.com/grpc-ecosystem/go-grpc-middleware/tags"
"github.com/pkg/errors"
"go.uber.org/multierr"
"go.uber.org/zap"
Expand All @@ -21,6 +18,7 @@ import (
"google.golang.org/grpc/health/grpc_health_v1"

"github.com/kakao/varlog/internal/admin/snwatcher"
"github.com/kakao/varlog/pkg/rpc/interceptors/logging"
"github.com/kakao/varlog/pkg/types"
"github.com/kakao/varlog/pkg/util/netutil"
"github.com/kakao/varlog/pkg/verrors"
Expand Down Expand Up @@ -72,13 +70,8 @@ func New(ctx context.Context, opts ...Option) (*Admin, error) {
}

grpcServer := grpc.NewServer(
grpcmiddleware.WithUnaryServerChain(
grpcctxtags.UnaryServerInterceptor(),
grpczap.UnaryServerInterceptor(cfg.logger, grpczap.WithDecider(
func(fullMethodName string, err error) bool {
return err != nil || !grpcHandlerLogDenyList[fullMethodName]
},
)),
grpc.ChainUnaryInterceptor(
logging.UnaryServerInterceptor(cfg.logger),
),
)

Expand Down
20 changes: 3 additions & 17 deletions internal/storagenode/storagenode.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package storagenode
import (
"context"
"errors"
"fmt"
"net"
"net/http"
"os"
Expand All @@ -13,7 +12,6 @@ import (
"sync/atomic"
"time"

"github.com/gogo/status"
"github.com/puzpuzpuz/xsync/v2"
"github.com/soheilhy/cmux"
"go.opentelemetry.io/otel"
Expand All @@ -32,6 +30,7 @@ import (
"github.com/kakao/varlog/internal/storagenode/pprof"
"github.com/kakao/varlog/internal/storagenode/telemetry"
"github.com/kakao/varlog/internal/storagenode/volume"
"github.com/kakao/varlog/pkg/rpc/interceptors/logging"
"github.com/kakao/varlog/pkg/types"
"github.com/kakao/varlog/pkg/util/fputil"
"github.com/kakao/varlog/pkg/util/netutil"
Expand Down Expand Up @@ -105,21 +104,8 @@ func NewStorageNode(opts ...Option) (*StorageNode, error) {
grpc.ReadBufferSize(int(cfg.grpcServerReadBufferSize)),
grpc.WriteBufferSize(int(cfg.grpcServerWriteBufferSize)),
grpc.MaxRecvMsgSize(int(cfg.grpcServerMaxRecvMsgSize)),
grpc.UnaryInterceptor(
func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
start := time.Now()
resp, err := handler(ctx, req)
if err != nil {
duration := time.Since(start)
cfg.logger.Error(info.FullMethod,
zap.Stringer("code", status.Code(err)),
zap.Int64("duration", duration.Microseconds()),
zap.Stringer("request", req.(fmt.Stringer)),
zap.Error(err),
)
}
return resp, err
},
grpc.ChainUnaryInterceptor(
logging.UnaryServerInterceptor(cfg.logger),
),
}
if opt := cfg.grpcServerInitialConnWindowSize; opt.set {
Expand Down
57 changes: 57 additions & 0 deletions pkg/rpc/interceptors/logging/interceptor.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package logging

import (
"context"
"fmt"
"net"
"time"

"go.uber.org/zap"
"google.golang.org/grpc"
"google.golang.org/grpc/peer"
"google.golang.org/grpc/status"
)

// UnaryServerInterceptor returns a new unary server interceptor that logs a
// gRPC error.
func UnaryServerInterceptor(logger *zap.Logger) grpc.UnaryServerInterceptor {
if logger == nil {
return func(ctx context.Context, req interface{}, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp interface{}, err error) {
return handler(ctx, req)
}
}

return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp interface{}, err error) {
start := time.Now()
resp, err = handler(ctx, req)
if err != nil {
duration := time.Since(start)
logger.Error(info.FullMethod,
zap.Stringer("code", status.Code(err)),
zap.Duration("duration", duration),
zap.Stringer("request", req.(fmt.Stringer)),
zap.Stringer("response", resp.(fmt.Stringer)),
zap.String("peer", peerAddr(ctx)),
zap.Error(err),
)
}
return resp, err
}
}

func peerAddr(ctx context.Context) string {
p, ok := peer.FromContext(ctx)
if !ok {
return ""
}

host, _, err := net.SplitHostPort(p.Addr.String())
if err != nil {
return ""
}

if host == "" {
return "127.0.0.1"
}
return host
}
204 changes: 0 additions & 204 deletions vendor/github.com/grpc-ecosystem/go-grpc-middleware/.gitignore

This file was deleted.

Loading

0 comments on commit e9b46ad

Please sign in to comment.