Removed tags; Simplified interceptor code; Added logging fields edita…

…bility. Fixes #382 Signed-off-by: Bartlomiej Plotka <[email protected]> # Conflicts: # go.sum # interceptors/client_test.go # interceptors/logging/interceptors.go # interceptors/logging/payload.go # interceptors/logging/payload_test.go # interceptors/tags/context.go # interceptors/tags/doc.go # interceptors/tags/examples_test.go # interceptors/tags/fieldextractor.go # interceptors/tags/fieldextractor_test.go # interceptors/tags/interceptors.go # interceptors/tags/interceptors_test.go # interceptors/tags/options.go
grpc-ecosystem · Aug 6, 2021 · 7dca0de · 7dca0de
1 parent 7d56e76
commit 7dca0de
Show file tree

Hide file tree

Showing 39 changed files with 576 additions and 1,126 deletions.
diff --git a/.gitignore b/.gitignore
@@ -202,4 +202,4 @@ coverage.txt
 vendor/
 
 .envrc
-.bin
+.bin
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,12 @@ Types of changes:
 - `Fixed` for any bug fixes.
 - `Security` in case of vulnerabilities.
 
+## v2 
+
+### Changed 
+
+* `tags` removed. Use `logging.ExtractFields` to read logging fields from logging interceptor for your local request logger. Use `logging.InjectFields` to inject custom fields to logging interceptor to client context or interceptor before logging interceptor.
+
 ## [Unreleased]
 
 ### Added

diff --git a/go.sum b/go.sum
@@ -102,6 +102,7 @@ github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm4
 github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg=
 github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk=
+github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 h1:+9834+KizmvFV7pXQGSXQTsaWhq2GjuNUt0aUU0YBYw=
 github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
 github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
 github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=

diff --git a/interceptors/auth/examples_test.go b/interceptors/auth/examples_test.go
@@ -12,8 +12,9 @@ import (
 	pb "google.golang.org/grpc/examples/helloworld/helloworld"
 	"google.golang.org/grpc/status"
 
+	"github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/logging"
+
 	"github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/auth"
-	"github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tags"
 )
 
 var tokenInfoKey struct{}
@@ -38,12 +39,10 @@ func exampleAuthFunc(ctx context.Context) (context.Context, error) {
 		return nil, status.Errorf(codes.Unauthenticated, "invalid auth token: %v", err)
 	}
 
-	tags.Extract(ctx).Set("auth.sub", userClaimFromToken(tokenInfo))
-
-	// WARNING: in production define your own type to avoid context collisions
-	newCtx := context.WithValue(ctx, tokenInfoKey, tokenInfo)
+	ctx = logging.InjectFields(ctx, logging.Fields{"auth.sub", userClaimFromToken(tokenInfo)})
 
-	return newCtx, nil
+	// WARNING: In production define your own type to avoid context collisions.
+	return context.WithValue(ctx, tokenInfoKey, tokenInfo), nil
 }
 
 // Simple example of server initialization code

diff --git a/interceptors/client.go b/interceptors/client.go
@@ -17,7 +17,7 @@ import (
 func UnaryClientInterceptor(reportable ClientReportable) grpc.UnaryClientInterceptor {
 	return func(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error {
 		r := newReport(Unary, method)
-		reporter, newCtx := reportable.ClientReporter(ctx, req, r.rpcType, r.service, r.method)
+		reporter, newCtx := reportable.ClientReporter(ctx, CallMeta{ReqProtoOrNil: req, Typ: r.rpcType, Service: r.service, Method: r.method})
 
 		reporter.PostMsgSend(req, nil, time.Since(r.startTime))
 		err := invoker(newCtx, method, req, reply, cc, opts...)
@@ -32,7 +32,7 @@ func UnaryClientInterceptor(reportable ClientReportable) grpc.UnaryClientInterce
 func StreamClientInterceptor(reportable ClientReportable) grpc.StreamClientInterceptor {
 	return func(ctx context.Context, desc *grpc.StreamDesc, cc *grpc.ClientConn, method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) {
 		r := newReport(clientStreamType(desc), method)
-		reporter, newCtx := reportable.ClientReporter(ctx, nil, r.rpcType, r.service, r.method)
+		reporter, newCtx := reportable.ClientReporter(ctx, CallMeta{ReqProtoOrNil: nil, Typ: r.rpcType, Service: r.service, Method: r.method})
 
 		clientStream, err := streamer(newCtx, desc, cc, method, opts...)
 		if err != nil {

diff --git a/interceptors/client_test.go b/interceptors/client_test.go
@@ -21,8 +21,7 @@ import (
 )
 
 type mockReport struct {
-	typ                 GRPCType
-	svcName, methodName string
+	CallMeta
 
 	postCalls       []error
 	postMsgSends    []error
@@ -40,9 +39,9 @@ type mockReportable struct {
 func (m *mockReportable) Equal(t *testing.T, expected []*mockReport) {
 	require.Len(t, expected, len(m.reports))
 	for i, e := range m.reports {
-		require.Equal(t, expected[i].typ, e.typ, "%v", i)
-		require.Equal(t, expected[i].svcName, e.svcName, "%v", i)
-		require.Equal(t, expected[i].methodName, e.methodName, "%v", i)
+		require.Equal(t, expected[i].Typ, e.Typ, "%v", i)
+		require.Equal(t, expected[i].Service, e.Service, "%v", i)
+		require.Equal(t, expected[i].Method, e.Method, "%v", i)
 
 		require.Len(t, expected[i].postCalls, len(e.postCalls), "%v", i)
 		for k, err := range e.postCalls {
@@ -111,14 +110,14 @@ func (m *mockReportable) PostMsgReceive(_ interface{}, err error, _ time.Duratio
 	m.curr.postMsgReceives = append(m.curr.postMsgReceives, err)
 }
 
-func (m *mockReportable) ClientReporter(ctx context.Context, _ interface{}, typ GRPCType, serviceName string, methodName string) (Reporter, context.Context) {
-	m.curr = &mockReport{typ: typ, svcName: serviceName, methodName: methodName}
+func (m *mockReportable) ClientReporter(ctx context.Context, c CallMeta) (Reporter, context.Context) {
+	m.curr = &mockReport{CallMeta: c}
 	m.reports = append(m.reports, m.curr)
 	return m, ctx
 }
 
-func (m *mockReportable) ServerReporter(ctx context.Context, _ interface{}, typ GRPCType, serviceName string, methodName string) (Reporter, context.Context) {
-	m.curr = &mockReport{typ: typ, svcName: serviceName, methodName: methodName}
+func (m *mockReportable) ServerReporter(ctx context.Context, c CallMeta) (Reporter, context.Context) {
+	m.curr = &mockReport{CallMeta: c}
 	m.reports = append(m.reports, m.curr)
 	return m, ctx
 }
@@ -206,9 +205,7 @@ func (s *ClientInterceptorTestSuite) TestUnaryReporting() {
 	_, err := s.testClient.PingEmpty(s.ctx, &testpb.PingEmptyRequest{}) // should return with code=OK
 	require.NoError(s.T(), err)
 	s.mock.Equal(s.T(), []*mockReport{{
-		typ:             Unary,
-		svcName:         testpb.TestServiceFullName,
-		methodName:      "PingEmpty",
+		CallMeta:        CallMeta{Typ: Unary, Service: testpb.TestServiceFullName, Method: "PingEmpty"},
 		postCalls:       []error{nil},
 		postMsgReceives: []error{nil},
 		postMsgSends:    []error{nil},
@@ -218,9 +215,7 @@ func (s *ClientInterceptorTestSuite) TestUnaryReporting() {
 	_, err = s.testClient.PingError(s.ctx, &testpb.PingErrorRequest{ErrorCodeReturned: uint32(codes.FailedPrecondition)}) // should return with code=FailedPrecondition
 	require.Error(s.T(), err)
 	s.mock.Equal(s.T(), []*mockReport{{
-		typ:             Unary,
-		svcName:         testpb.TestServiceFullName,
-		methodName:      "PingError",
+		CallMeta:        CallMeta{Typ: Unary, Service: testpb.TestServiceFullName, Method: "PingError"},
 		postCalls:       []error{status.Errorf(codes.FailedPrecondition, "Userspace error.")},
 		postMsgReceives: []error{status.Errorf(codes.FailedPrecondition, "Userspace error.")},
 		postMsgSends:    []error{nil},
@@ -233,9 +228,7 @@ func (s *ClientInterceptorTestSuite) TestStartedListReporting() {
 
 	// Even without reading, we should get initial mockReport.
 	s.mock.Equal(s.T(), []*mockReport{{
-		typ:          ServerStream,
-		svcName:      testpb.TestServiceFullName,
-		methodName:   "PingList",
+		CallMeta:     CallMeta{Typ: ServerStream, Service: testpb.TestServiceFullName, Method: "PingList"},
 		postMsgSends: []error{nil},
 	}})
 
@@ -244,14 +237,10 @@ func (s *ClientInterceptorTestSuite) TestStartedListReporting() {
 
 	// Even without reading, we should get initial mockReport.
 	s.mock.Equal(s.T(), []*mockReport{{
-		typ:          ServerStream,
-		svcName:      testpb.TestServiceFullName,
-		methodName:   "PingList",
+		CallMeta:     CallMeta{Typ: ServerStream, Service: testpb.TestServiceFullName, Method: "PingList"},
 		postMsgSends: []error{nil},
 	}, {
-		typ:          ServerStream,
-		svcName:      testpb.TestServiceFullName,
-		methodName:   "PingList",
+		CallMeta:     CallMeta{Typ: ServerStream, Service: testpb.TestServiceFullName, Method: "PingList"},
 		postMsgSends: []error{nil},
 	}})
 }
@@ -273,10 +262,8 @@ func (s *ClientInterceptorTestSuite) TestListReporting() {
 	require.EqualValues(s.T(), testpb.ListResponseCount, count, "Number of received msg on the wire must match")
 
 	s.mock.Equal(s.T(), []*mockReport{{
-		typ:             ServerStream,
-		svcName:         testpb.TestServiceFullName,
-		methodName:      "PingList",
-		postCalls:       []error{nil},
+		CallMeta:        CallMeta{Typ: ServerStream, Service: testpb.TestServiceFullName, Method: "PingList"},
+		postCalls:       []error{io.EOF},
 		postMsgReceives: append(make([]error, testpb.ListResponseCount), io.EOF),
 		postMsgSends:    []error{nil},
 	}})
@@ -298,9 +285,7 @@ func (s *ClientInterceptorTestSuite) TestListReporting() {
 	require.Equal(s.T(), codes.FailedPrecondition, st.Code(), "Recv must return FailedPrecondition, otherwise the test is wrong")
 
 	s.mock.Equal(s.T(), []*mockReport{{
-		typ:             ServerStream,
-		svcName:         testpb.TestServiceFullName,
-		methodName:      "PingList",
+		CallMeta:        CallMeta{Typ: ServerStream, Service: testpb.TestServiceFullName, Method: "PingList"},
 		postCalls:       []error{status.Errorf(codes.FailedPrecondition, "foobar"), status.Errorf(codes.FailedPrecondition, "foobar")},
 		postMsgReceives: []error{status.Errorf(codes.FailedPrecondition, "foobar"), status.Errorf(codes.FailedPrecondition, "foobar")},
 		postMsgSends:    []error{nil},
@@ -344,10 +329,8 @@ func (s *ClientInterceptorTestSuite) TestBiStreamingReporting() {
 
 	require.EqualValues(s.T(), 100, count, "Number of received msg on the wire must match")
 	s.mock.Equal(s.T(), []*mockReport{{
-		typ:             BidiStream,
-		svcName:         testpb.TestServiceFullName,
-		methodName:      "PingStream",
-		postCalls:       []error{nil},
+		CallMeta:        CallMeta{Typ: BidiStream, Service: testpb.TestServiceFullName, Method: "PingStream"},
+		postCalls:       []error{io.EOF},
 		postMsgReceives: append(make([]error, 100), io.EOF),
 		postMsgSends:    make([]error, 100),
 	}})

diff --git a/interceptors/logging/doc.go b/interceptors/logging/doc.go
@@ -4,19 +4,19 @@
 /*
 logging is a "parent" package for gRPC logging middlewares.
 
-The gRPC logging middleware populates request-scoped data to `grpc_ctxtags.Tags` that relate to the current gRPC call
-(e.g. service and method names).
+The gRPC logging middleware populates request-scoped data to `logging.Fields` that relate to the current gRPC call
+(e.g. service and method names). You can laverage that data using `logging.ExtractFields` and `logging.InjectFields`.
 
-Once the gRPC logging middleware has added the gRPC specific Tags to the ctx they will then be written with the logs
-that are made using the `ctx_logrus` or `ctx_zap` loggers.
+Once the gRPC logging middleware has added the gRPC specific Fields to the ctx they will then be written with the log lines.
 
 All logging middleware will emit a final log statement. It is based on the error returned by the handler function,
-the gRPC status code, an error (if any) and it emit at a level controlled via `WithLevels`.
+the gRPC status code, an error (if any) and it emit at a level controlled via `WithLevels`. You can control this behavior
+using `WithDecider`.
 
 This parent package
 
 This particular package is intended for use by other middleware, logging or otherwise. It contains interfaces that other
-logging middlewares *could* share . This allows code to be shared between different implementations.
+logging middlewares *could* share. This allows code to be shared between different implementations.
 
 Field names
 
@@ -31,6 +31,5 @@ Implementations:
 * providers/zerolog
 * providers/phuslog
 
-See relevant packages below.
 */
 package logging