Fix request id key (#197)

* fix request id key * fix formating * fix tests * add test whith deprecated request-id key
infobloxopen · Jul 27, 2020 · 55b3a20 · 55b3a20
1 parent bdfb656
commit 55b3a20
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 8 deletions.
diff --git a/logging/interceptor_test.go b/logging/interceptor_test.go
@@ -19,6 +19,7 @@ import (
 	"google.golang.org/grpc/metadata"
 
 	"github.com/infobloxopen/atlas-app-toolkit/logging/mocks"
+	"github.com/infobloxopen/atlas-app-toolkit/requestid"
 )
 
 const (
@@ -38,7 +39,7 @@ var (
 	buf         bytes.Buffer
 	reader      io.Reader
 	testLogger  = New("Info")
-	testMD      = metautils.NiceMD{}.Set(testAuthorizationHeader, testJWT).Set(DefaultRequestIDKey, testRequestID).Set(testCustomHeaderKey, testCustomHeaderVal)
+	testMD      = metautils.NiceMD{}.Set(testAuthorizationHeader, testJWT).Set(requestid.DefaultRequestIDKey, testRequestID).Set(testCustomHeaderKey, testCustomHeaderVal)
 	testSubject = map[string]interface{}{"id": "testID", "subject_type": "testUser", "authentication_type": "test"}
 )
 
@@ -68,7 +69,11 @@ func TestUnaryClientInterceptor(t *testing.T) {
 		newMD, ok := metadata.FromIncomingContext(ctx)
 		assert.True(t, ok)
 		assert.Equal(t, testJWT, newMD.Get(testAuthorizationHeader)[0])
-		assert.Equal(t, testRequestID, newMD.Get(DefaultRequestIDKey)[0])
+
+		reqID, ok := requestid.FromContext(ctx)
+		assert.True(t, ok)
+		assert.Equal(t, testRequestID, reqID)
+
 		assert.Equal(t, testMethod, method)
 
 		return nil
@@ -126,7 +131,11 @@ func TestStreamClientInterceptor(t *testing.T) {
 		newMD, ok := metadata.FromIncomingContext(ctx)
 		assert.True(t, ok)
 		assert.Equal(t, testJWT, newMD.Get(testAuthorizationHeader)[0])
-		assert.Equal(t, testRequestID, newMD.Get(DefaultRequestIDKey)[0])
+
+		reqID, ok := requestid.FromContext(ctx)
+		assert.True(t, ok)
+		assert.Equal(t, testRequestID, reqID)
+
 		assert.Equal(t, testMethod, method)
 
 		return nil, nil
@@ -186,7 +195,54 @@ func TestUnaryServerInterceptor(t *testing.T) {
 		newMD, ok := metadata.FromIncomingContext(ctx)
 		assert.True(t, ok)
 		assert.Equal(t, testJWT, newMD.Get(testAuthorizationHeader)[0])
-		assert.Equal(t, testRequestID, newMD.Get(DefaultRequestIDKey)[0])
+
+		reqID, ok := requestid.FromContext(ctx)
+		assert.True(t, ok)
+		assert.Equal(t, testRequestID, reqID)
+
+		entry := ctxlogrus.Extract(ctx)
+		assert.Equal(t, testRequestID, entry.Data[DefaultRequestIDKey])
+		assert.Equal(t, testAccID, entry.Data[DefaultAccountIDKey])
+		assert.Equal(t, testSubject, entry.Data[DefaultSubjectKey])
+
+		return nil, nil
+	}
+
+	resp, err := interceptor(ctx, nil, &grpc.UnaryServerInfo{FullMethod: testFullMethod}, handlerMock)
+	assert.NoError(t, err)
+	assert.Nil(t, resp)
+
+	reader = &buf
+	bts, err := ioutil.ReadAll(reader)
+	assert.NoError(t, err)
+
+	result := map[string]interface{}{}
+
+	err = json.Unmarshal(bts, &result)
+	assert.NoError(t, err)
+	assert.Equal(t, testAccID, result[DefaultAccountIDKey])
+	assert.Equal(t, testRequestID, result[DefaultRequestIDKey])
+	assert.Equal(t, testSubject, result[DefaultSubjectKey])
+	assert.Equal(t, "app.Object", result[DefaultGRPCServiceKey])
+	assert.Equal(t, testMethod, result[DefaultGRPCMethodKey])
+	assert.Equal(t, "finished unary call with code OK", result["msg"])
+}
+
+func TestUnaryServerInterceptorDeprecatedHeader(t *testing.T) {
+	testLogger.Out = &buf
+	interceptor := UnaryServerInterceptor(logrus.NewEntry(testLogger))
+
+	md := metautils.NiceMD{}.Set(testAuthorizationHeader, testJWT).Set(requestid.DeprecatedRequestIDKey, testRequestID).Set(testCustomHeaderKey, testCustomHeaderVal)
+	ctx := metadata.NewIncomingContext(context.Background(), metadata.MD(md))
+
+	handlerMock := func(ctx context.Context, req interface{}) (interface{}, error) {
+		newMD, ok := metadata.FromIncomingContext(ctx)
+		assert.True(t, ok)
+		assert.Equal(t, testJWT, newMD.Get(testAuthorizationHeader)[0])
+
+		reqID, ok := requestid.FromContext(ctx)
+		assert.True(t, ok)
+		assert.Equal(t, testRequestID, reqID)
 
 		entry := ctxlogrus.Extract(ctx)
 		assert.Equal(t, testRequestID, entry.Data[DefaultRequestIDKey])
@@ -251,7 +307,10 @@ func TestStreamServerInterceptor(t *testing.T) {
 		newMD, ok := metadata.FromIncomingContext(stream.Context())
 		assert.True(t, ok)
 		assert.Equal(t, testJWT, newMD.Get(testAuthorizationHeader)[0])
-		assert.Equal(t, testRequestID, newMD.Get(DefaultRequestIDKey)[0])
+
+		reqID, ok := requestid.FromContext(stream.Context())
+		assert.True(t, ok)
+		assert.Equal(t, testRequestID, reqID)
 
 		entry := ctxlogrus.Extract(stream.Context())
 		assert.Equal(t, testRequestID, entry.Data[DefaultRequestIDKey])

diff --git a/query/collection_operators.pb.go b/query/collection_operators.pb.go
diff --git a/requestid/interceptor.go b/requestid/interceptor.go
@@ -18,6 +18,7 @@ func UnaryServerInterceptor() grpc.UnaryServerInterceptor {
 	return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (res interface{}, err error) {
 
 		reqID := HandleRequestID(ctx)
+
 		// add request id to logger
 		addRequestIDToLogger(ctx, reqID)
 
@@ -36,6 +37,7 @@ func StreamServerInterceptor() grpc.StreamServerInterceptor {
 		ctx := stream.Context()
 
 		reqID := HandleRequestID(ctx)
+
 		// add request id to logger
 		addRequestIDToLogger(ctx, reqID)
 

diff --git a/requestid/requestid.go b/requestid/requestid.go
@@ -14,7 +14,8 @@ import (
 // DefaultRequestIDKey is the metadata key name for request ID
 const (
 	DeprecatedRequestIDKey = "Request-Id"
-	DefaultRequestIDKey    = "request_id"
+	DefaultRequestIDKey    = "X-Request-ID"
+	RequestIDLogKey        = "request_id"
 )
 
 // HandleRequestID either extracts a existing and valid request ID from the context or generates a new one
@@ -51,5 +52,5 @@ func NewContext(ctx context.Context, reqID string) context.Context {
 }
 
 func addRequestIDToLogger(ctx context.Context, reqID string) {
-	ctxlogrus.AddFields(ctx, logrus.Fields{DefaultRequestIDKey: reqID})
+	ctxlogrus.AddFields(ctx, logrus.Fields{RequestIDLogKey: reqID})
 }