Skip to content

Commit

Permalink
Use empty namespace metrics tag if namespace not found (#3686)
Browse files Browse the repository at this point in the history
* Use empty namespace metrics tag if namespace not found
  • Loading branch information
yux0 committed Dec 5, 2022
1 parent 21ed2aa commit 4907ff9
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 12 deletions.
7 changes: 6 additions & 1 deletion common/rpc/interceptor/caller_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ import (
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"go.temporal.io/api/workflowservice/v1"
"google.golang.org/grpc"

"go.temporal.io/server/common/headers"
"go.temporal.io/server/common/namespace"
"google.golang.org/grpc"
)

type (
Expand Down Expand Up @@ -71,6 +72,7 @@ func (s *callerInfoSuite) TestIntercept_CallerName() {
// testNamespaceID := namespace.NewID()
testNamespaceName := namespace.Name("test-namespace")
// s.mockRegistry.EXPECT().GetNamespaceName(testNamespaceID).Return(testNamespaceName, nil).AnyTimes()
s.mockRegistry.EXPECT().GetNamespace(gomock.Any()).Return(nil, nil).AnyTimes()

testCases := []struct {
setupIncomingCtx func() context.Context
Expand Down Expand Up @@ -139,6 +141,8 @@ func (s *callerInfoSuite) TestIntercept_CallerName() {
}

func (s *callerInfoSuite) TestIntercept_CallerType() {
s.mockRegistry.EXPECT().GetNamespace(gomock.Any()).Return(nil, nil).AnyTimes()

testCases := []struct {
setupIncomingCtx func() context.Context
request interface{}
Expand Down Expand Up @@ -202,6 +206,7 @@ func (s *callerInfoSuite) TestIntercept_CallOrigin() {
serverInfo := &grpc.UnaryServerInfo{
FullMethod: "/temporal/" + method,
}
s.mockRegistry.EXPECT().GetNamespace(gomock.Any()).Return(nil, nil).AnyTimes()

testCases := []struct {
setupIncomingCtx func() context.Context
Expand Down
11 changes: 8 additions & 3 deletions common/rpc/interceptor/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,22 @@ func GetNamespace(
) namespace.Name {
switch request := req.(type) {
case NamespaceNameGetter:
return namespace.Name(request.GetNamespace())
namespaceName := namespace.Name(request.GetNamespace())
_, err := namespaceRegistry.GetNamespace(namespaceName)
if err != nil {
return namespace.EmptyName
}
return namespaceName

case NamespaceIDGetter:
namespaceID := namespace.ID(request.GetNamespaceId())
namespaceName, err := namespaceRegistry.GetNamespaceName(namespaceID)
if err != nil {
return ""
return namespace.EmptyName
}
return namespaceName

default:
return ""
return namespace.EmptyName
}
}
53 changes: 45 additions & 8 deletions common/rpc/interceptor/namespace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,23 @@
package interceptor

import (
"errors"
"fmt"
"reflect"
"testing"

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"go.temporal.io/api/workflowservice/v1"

"go.temporal.io/server/api/historyservice/v1"
"go.temporal.io/server/api/matchingservice/v1"
"go.temporal.io/server/common/namespace"
)

type (
nameepaceSuite struct {
namespaceSuite struct {
suite.Suite
*require.Assertions
}
Expand Down Expand Up @@ -70,20 +73,20 @@ var (
}
)

func TestNameepaceSuite(t *testing.T) {
s := new(nameepaceSuite)
func TestNamespaceSuite(t *testing.T) {
s := new(namespaceSuite)
suite.Run(t, s)
}

func (s *nameepaceSuite) SetupTest() {
func (s *namespaceSuite) SetupTest() {
s.Assertions = require.New(s.T())
}

func (s *nameepaceSuite) TearDownTest() {
func (s *namespaceSuite) TearDownTest() {

}

func (s *nameepaceSuite) TestFrontendAPIMetrics() {
func (s *namespaceSuite) TestFrontendAPIMetrics() {
namespaceNameGetter := reflect.TypeOf((*NamespaceNameGetter)(nil)).Elem()

var service workflowservice.WorkflowServiceServer
Expand All @@ -108,7 +111,7 @@ func (s *nameepaceSuite) TestFrontendAPIMetrics() {
}
}

func (s *nameepaceSuite) TestMatchingAPIMetrics() {
func (s *namespaceSuite) TestMatchingAPIMetrics() {
namespaceIDGetter := reflect.TypeOf((*NamespaceIDGetter)(nil)).Elem()

var service matchingservice.MatchingServiceServer
Expand All @@ -133,7 +136,7 @@ func (s *nameepaceSuite) TestMatchingAPIMetrics() {
}
}

func (s *nameepaceSuite) TestHistoryAPIMetrics() {
func (s *namespaceSuite) TestHistoryAPIMetrics() {
namespaceIDGetter := reflect.TypeOf((*NamespaceIDGetter)(nil)).Elem()

var service historyservice.HistoryServiceServer
Expand All @@ -157,3 +160,37 @@ func (s *nameepaceSuite) TestHistoryAPIMetrics() {
}
}
}

func (s *namespaceSuite) TestGetNamespace() {
register := namespace.NewMockRegistry(gomock.NewController(s.T()))
register.EXPECT().GetNamespace(namespace.Name("exist")).Return(nil, nil)
register.EXPECT().GetNamespace(namespace.Name("nonexist")).Return(nil, errors.New("not found"))
register.EXPECT().GetNamespaceName(namespace.ID("exist")).Return(namespace.Name("exist"), nil)
register.EXPECT().GetNamespaceName(namespace.ID("nonexist")).Return(namespace.EmptyName, errors.New("not found"))
testCases := []struct {
method interface{}
namespaceName namespace.Name
}{
{
&workflowservice.DescribeNamespaceRequest{Namespace: "exist"},
namespace.Name("exist"),
},
{
&workflowservice.DescribeNamespaceRequest{Namespace: "nonexist"},
namespace.EmptyName,
},
{
&historyservice.DescribeMutableStateRequest{NamespaceId: "exist"},
namespace.Name("exist"),
},
{
&historyservice.DescribeMutableStateRequest{NamespaceId: "nonexist"},
namespace.EmptyName,
},
}

for _, testCase := range testCases {
extractedNamespace := GetNamespace(register, testCase.method)
s.Equal(testCase.namespaceName, extractedNamespace)
}
}

0 comments on commit 4907ff9

Please sign in to comment.