From 882f6808a731093018789fc0925439960504d389 Mon Sep 17 00:00:00 2001 From: Bowei Xu Date: Fri, 27 Mar 2020 11:59:48 -0700 Subject: [PATCH] Add metrics to AccessControlledHandler (#3145) --- common/metrics/defs.go | 7 + service/frontend/accessControlledHandler.go | 147 ++++++++++++++---- .../frontend/accessControlledHandler_test.go | 114 ++++++++++++++ service/frontend/dcRedirectionHandler_test.go | 2 +- service/frontend/workflowHandler.go | 7 +- 5 files changed, 240 insertions(+), 37 deletions(-) create mode 100644 service/frontend/accessControlledHandler_test.go diff --git a/common/metrics/defs.go b/common/metrics/defs.go index 9b4c610702a..2eab6160fde 100644 --- a/common/metrics/defs.go +++ b/common/metrics/defs.go @@ -1521,6 +1521,8 @@ const ( CadenceErrClientVersionNotSupportedCounter CadenceErrIncompleteHistoryCounter CadenceErrNonDeterministicCounter + CadenceErrUnauthorizedCounter + CadenceErrAuthorizeFailedCounter PersistenceRequests PersistenceFailures PersistenceLatency @@ -1544,6 +1546,8 @@ const ( CadenceDcRedirectionClientFailures CadenceDcRedirectionClientLatency + CadenceAuthorizationLatency + DomainCachePrepareCallbacksLatency DomainCacheCallbacksLatency @@ -1890,6 +1894,8 @@ var MetricDefs = map[ServiceIdx]map[int]metricDefinition{ CadenceErrClientVersionNotSupportedCounter: {metricName: "cadence_errors_client_version_not_supported", metricType: Counter}, CadenceErrIncompleteHistoryCounter: {metricName: "cadence_errors_incomplete_history", metricType: Counter}, CadenceErrNonDeterministicCounter: {metricName: "cadence_errors_nondeterministic", metricType: Counter}, + CadenceErrUnauthorizedCounter: {metricName: "cadence_errors_unauthorized", metricType: Counter}, + CadenceErrAuthorizeFailedCounter: {metricName: "cadence_errors_authorize_failed", metricType: Counter}, PersistenceRequests: {metricName: "persistence_requests", metricType: Counter}, PersistenceFailures: {metricName: "persistence_errors", metricType: Counter}, PersistenceLatency: {metricName: "persistence_latency", metricType: Timer}, @@ -1910,6 +1916,7 @@ var MetricDefs = map[ServiceIdx]map[int]metricDefinition{ CadenceDcRedirectionClientRequests: {metricName: "cadence_client_requests_redirection", metricType: Counter}, CadenceDcRedirectionClientFailures: {metricName: "cadence_client_errors_redirection", metricType: Counter}, CadenceDcRedirectionClientLatency: {metricName: "cadence_client_latency_redirection", metricType: Timer}, + CadenceAuthorizationLatency: {metricName: "cadence_authorization_latency", metricType: Timer}, DomainCachePrepareCallbacksLatency: {metricName: "domain_cache_prepare_callbacks_latency", metricType: Timer}, DomainCacheCallbacksLatency: {metricName: "domain_cache_callbacks_latency", metricType: Timer}, HistorySize: {metricName: "history_size", metricType: Timer}, diff --git a/service/frontend/accessControlledHandler.go b/service/frontend/accessControlledHandler.go index 12f93dbde7f..3e09ef92e9c 100644 --- a/service/frontend/accessControlledHandler.go +++ b/service/frontend/accessControlledHandler.go @@ -23,17 +23,15 @@ package frontend import ( "context" - "github.com/uber/cadence/common/authorization" - "github.com/uber/cadence/.gen/go/cadence/workflowserviceserver" "github.com/uber/cadence/.gen/go/health" "github.com/uber/cadence/.gen/go/health/metaserver" "github.com/uber/cadence/.gen/go/shared" + "github.com/uber/cadence/common/authorization" + "github.com/uber/cadence/common/metrics" "github.com/uber/cadence/common/resource" ) -// TODO(vancexu): add metrics - var errUnauthorized = &shared.BadRequestError{Message: "Request unauthorized."} // AccessControlledWorkflowHandler frontend handler wrapper for authentication and authorization @@ -100,11 +98,13 @@ func (a *AccessControlledWorkflowHandler) CountWorkflowExecutions( request *shared.CountWorkflowExecutionsRequest, ) (*shared.CountWorkflowExecutionsResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendCountWorkflowExecutionsScope, request) + attr := &authorization.Attributes{ APIName: "CountWorkflowExecutions", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -121,11 +121,13 @@ func (a *AccessControlledWorkflowHandler) DeprecateDomain( request *shared.DeprecateDomainRequest, ) error { + scope := a.getMetricsScopeWithDomainName(metrics.FrontendDeprecateDomainScope, request.GetName()) + attr := &authorization.Attributes{ APIName: "DeprecateDomain", DomainName: request.GetName(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return err } @@ -142,11 +144,13 @@ func (a *AccessControlledWorkflowHandler) DescribeDomain( request *shared.DescribeDomainRequest, ) (*shared.DescribeDomainResponse, error) { + scope := a.getMetricsScopeWithDomainName(metrics.FrontendDescribeDomainScope, request.GetName()) + attr := &authorization.Attributes{ APIName: "DescribeDomain", DomainName: request.GetName(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -163,11 +167,13 @@ func (a *AccessControlledWorkflowHandler) DescribeTaskList( request *shared.DescribeTaskListRequest, ) (*shared.DescribeTaskListResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendDescribeTaskListScope, request) + attr := &authorization.Attributes{ APIName: "DescribeTaskList", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -184,11 +190,13 @@ func (a *AccessControlledWorkflowHandler) DescribeWorkflowExecution( request *shared.DescribeWorkflowExecutionRequest, ) (*shared.DescribeWorkflowExecutionResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendDescribeWorkflowExecutionScope, request) + attr := &authorization.Attributes{ APIName: "DescribeWorkflowExecution", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -212,11 +220,13 @@ func (a *AccessControlledWorkflowHandler) GetWorkflowExecutionHistory( request *shared.GetWorkflowExecutionHistoryRequest, ) (*shared.GetWorkflowExecutionHistoryResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendGetWorkflowExecutionHistoryScope, request) + attr := &authorization.Attributes{ APIName: "GetWorkflowExecutionHistory", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -233,11 +243,13 @@ func (a *AccessControlledWorkflowHandler) ListArchivedWorkflowExecutions( request *shared.ListArchivedWorkflowExecutionsRequest, ) (*shared.ListArchivedWorkflowExecutionsResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendListArchivedWorkflowExecutionsScope, request) + attr := &authorization.Attributes{ APIName: "ListArchivedWorkflowExecutions", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -254,11 +266,13 @@ func (a *AccessControlledWorkflowHandler) ListClosedWorkflowExecutions( request *shared.ListClosedWorkflowExecutionsRequest, ) (*shared.ListClosedWorkflowExecutionsResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendListClosedWorkflowExecutionsScope, request) + attr := &authorization.Attributes{ APIName: "ListClosedWorkflowExecutions", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -275,10 +289,12 @@ func (a *AccessControlledWorkflowHandler) ListDomains( request *shared.ListDomainsRequest, ) (*shared.ListDomainsResponse, error) { + scope := a.GetResource().GetMetricsClient().Scope(metrics.FrontendListDomainsScope) + attr := &authorization.Attributes{ APIName: "ListDomains", } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -295,11 +311,13 @@ func (a *AccessControlledWorkflowHandler) ListOpenWorkflowExecutions( request *shared.ListOpenWorkflowExecutionsRequest, ) (*shared.ListOpenWorkflowExecutionsResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendListOpenWorkflowExecutionsScope, request) + attr := &authorization.Attributes{ APIName: "ListOpenWorkflowExecutions", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -316,11 +334,13 @@ func (a *AccessControlledWorkflowHandler) ListWorkflowExecutions( request *shared.ListWorkflowExecutionsRequest, ) (*shared.ListWorkflowExecutionsResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendListWorkflowExecutionsScope, request) + attr := &authorization.Attributes{ APIName: "ListWorkflowExecutions", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -337,11 +357,13 @@ func (a *AccessControlledWorkflowHandler) PollForActivityTask( request *shared.PollForActivityTaskRequest, ) (*shared.PollForActivityTaskResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendPollForActivityTaskScope, request) + attr := &authorization.Attributes{ APIName: "PollForActivityTask", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -358,11 +380,13 @@ func (a *AccessControlledWorkflowHandler) PollForDecisionTask( request *shared.PollForDecisionTaskRequest, ) (*shared.PollForDecisionTaskResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendPollForDecisionTaskScope, request) + attr := &authorization.Attributes{ APIName: "PollForDecisionTask", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -379,11 +403,13 @@ func (a *AccessControlledWorkflowHandler) QueryWorkflow( request *shared.QueryWorkflowRequest, ) (*shared.QueryWorkflowResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendQueryWorkflowScope, request) + attr := &authorization.Attributes{ APIName: "QueryWorkflow", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -424,11 +450,13 @@ func (a *AccessControlledWorkflowHandler) RegisterDomain( request *shared.RegisterDomainRequest, ) error { + scope := a.getMetricsScopeWithDomainName(metrics.FrontendRegisterDomainScope, request.GetName()) + attr := &authorization.Attributes{ APIName: "RegisterDomain", DomainName: request.GetName(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return err } @@ -445,11 +473,13 @@ func (a *AccessControlledWorkflowHandler) RequestCancelWorkflowExecution( request *shared.RequestCancelWorkflowExecutionRequest, ) error { + scope := a.getMetricsScopeWithDomain(metrics.FrontendRequestCancelWorkflowExecutionScope, request) + attr := &authorization.Attributes{ APIName: "RequestCancelWorkflowExecution", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return err } @@ -466,11 +496,13 @@ func (a *AccessControlledWorkflowHandler) ResetStickyTaskList( request *shared.ResetStickyTaskListRequest, ) (*shared.ResetStickyTaskListResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendResetStickyTaskListScope, request) + attr := &authorization.Attributes{ APIName: "ResetStickyTaskList", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -487,11 +519,13 @@ func (a *AccessControlledWorkflowHandler) ResetWorkflowExecution( request *shared.ResetWorkflowExecutionRequest, ) (*shared.ResetWorkflowExecutionResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendResetWorkflowExecutionScope, request) + attr := &authorization.Attributes{ APIName: "ResetWorkflowExecution", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -580,11 +614,13 @@ func (a *AccessControlledWorkflowHandler) ScanWorkflowExecutions( request *shared.ListWorkflowExecutionsRequest, ) (*shared.ListWorkflowExecutionsResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendScanWorkflowExecutionsScope, request) + attr := &authorization.Attributes{ APIName: "ScanWorkflowExecutions", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -601,11 +637,13 @@ func (a *AccessControlledWorkflowHandler) SignalWithStartWorkflowExecution( request *shared.SignalWithStartWorkflowExecutionRequest, ) (*shared.StartWorkflowExecutionResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendSignalWithStartWorkflowExecutionScope, request) + attr := &authorization.Attributes{ APIName: "SignalWithStartWorkflowExecution", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -622,11 +660,13 @@ func (a *AccessControlledWorkflowHandler) SignalWorkflowExecution( request *shared.SignalWorkflowExecutionRequest, ) error { + scope := a.getMetricsScopeWithDomain(metrics.FrontendSignalWorkflowExecutionScope, request) + attr := &authorization.Attributes{ APIName: "SignalWorkflowExecution", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return err } @@ -643,11 +683,13 @@ func (a *AccessControlledWorkflowHandler) StartWorkflowExecution( request *shared.StartWorkflowExecutionRequest, ) (*shared.StartWorkflowExecutionResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendStartWorkflowExecutionScope, request) + attr := &authorization.Attributes{ APIName: "StartWorkflowExecution", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -664,11 +706,13 @@ func (a *AccessControlledWorkflowHandler) TerminateWorkflowExecution( request *shared.TerminateWorkflowExecutionRequest, ) error { + scope := a.getMetricsScopeWithDomain(metrics.FrontendTerminateWorkflowExecutionScope, request) + attr := &authorization.Attributes{ APIName: "TerminateWorkflowExecution", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return err } @@ -685,11 +729,13 @@ func (a *AccessControlledWorkflowHandler) ListTaskListPartitions( request *shared.ListTaskListPartitionsRequest, ) (*shared.ListTaskListPartitionsResponse, error) { + scope := a.getMetricsScopeWithDomain(metrics.FrontendListTaskListPartitionsScope, request) + attr := &authorization.Attributes{ APIName: "ListTaskListPartitions", DomainName: request.GetDomain(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -706,11 +752,13 @@ func (a *AccessControlledWorkflowHandler) UpdateDomain( request *shared.UpdateDomainRequest, ) (*shared.UpdateDomainResponse, error) { + scope := a.getMetricsScopeWithDomainName(metrics.FrontendUpdateDomainScope, request.GetName()) + attr := &authorization.Attributes{ APIName: "UpdateDomain", DomainName: request.GetName(), } - isAuthorized, err := a.isAuthorized(ctx, attr) + isAuthorized, err := a.isAuthorized(ctx, attr, scope) if err != nil { return nil, err } @@ -724,10 +772,49 @@ func (a *AccessControlledWorkflowHandler) UpdateDomain( func (a *AccessControlledWorkflowHandler) isAuthorized( ctx context.Context, attr *authorization.Attributes, + scope metrics.Scope, ) (bool, error) { + sw := scope.StartTimer(metrics.CadenceAuthorizationLatency) + defer sw.Stop() + result, err := a.authorizer.Authorize(ctx, attr) if err != nil { + scope.IncCounter(metrics.CadenceErrAuthorizeFailedCounter) return false, err } - return result.Decision == authorization.DecisionAllow, nil + isAuth := result.Decision == authorization.DecisionAllow + if !isAuth { + scope.IncCounter(metrics.CadenceErrUnauthorizedCounter) + } + return isAuth, nil +} + +// getMetricsScopeWithDomain return metrics scope with domain tag +func (a *AccessControlledWorkflowHandler) getMetricsScopeWithDomain( + scope int, + d domainGetter, +) metrics.Scope { + return getMetricsScopeWithDomain(scope, d, a.GetResource().GetMetricsClient()) +} + +func getMetricsScopeWithDomain( + scope int, + d domainGetter, + metricsClient metrics.Client, +) metrics.Scope { + var metricsScope metrics.Scope + if d != nil { + metricsScope = metricsClient.Scope(scope).Tagged(metrics.DomainTag(d.GetDomain())) + } else { + metricsScope = metricsClient.Scope(scope).Tagged(metrics.DomainUnknownTag()) + } + return metricsScope +} + +// getMetricsScopeWithDomainName is for XXXDomain APIs, whose request is not domainGetter +func (a *AccessControlledWorkflowHandler) getMetricsScopeWithDomainName( + scope int, + domainName string, +) metrics.Scope { + return a.GetResource().GetMetricsClient().Scope(scope).Tagged(metrics.DomainTag(domainName)) } diff --git a/service/frontend/accessControlledHandler_test.go b/service/frontend/accessControlledHandler_test.go new file mode 100644 index 00000000000..ce78731655f --- /dev/null +++ b/service/frontend/accessControlledHandler_test.go @@ -0,0 +1,114 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package frontend + +import ( + "context" + "errors" + "testing" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + + "github.com/uber/cadence/common/authorization" + "github.com/uber/cadence/common/metrics" + "github.com/uber/cadence/common/metrics/mocks" +) + +type ( + accessControlledHandlerSuite struct { + suite.Suite + *require.Assertions + + controller *gomock.Controller + mockFrontendHandler *MockHandler + mockAuthorizer *authorization.MockAuthorizer + mockMetricsScope *mocks.Scope + + handler *AccessControlledWorkflowHandler + } +) + +func TestAccessControlledHandlerSuite(t *testing.T) { + s := new(accessControlledHandlerSuite) + suite.Run(t, s) +} + +func (s *accessControlledHandlerSuite) SetupTest() { + s.Assertions = require.New(s.T()) + s.controller = gomock.NewController(s.T()) + s.mockFrontendHandler = NewMockHandler(s.controller) + s.mockAuthorizer = authorization.NewMockAuthorizer(s.controller) + s.mockMetricsScope = &mocks.Scope{} + s.handler = NewAccessControlledHandlerImpl(s.mockFrontendHandler, s.mockAuthorizer) +} + +func (s *accessControlledHandlerSuite) TearDownTest() { + s.controller.Finish() + s.mockMetricsScope.AssertExpectations(s.T()) +} + +func (s *accessControlledHandlerSuite) TestIsAuthorized() { + ctx := context.Background() + attr := &authorization.Attributes{} + + s.mockMetricsScope.On("StartTimer", metrics.CadenceAuthorizationLatency). + Return(metrics.Stopwatch{}).Once() + s.mockAuthorizer.EXPECT().Authorize(ctx, attr). + Return(authorization.Result{Decision: authorization.DecisionAllow}, nil).Times(1) + + res, err := s.handler.isAuthorized(ctx, attr, s.mockMetricsScope) + s.True(res) + s.NoError(err) +} + +func (s *accessControlledHandlerSuite) TestIsAuthorized_Failed() { + ctx := context.Background() + attr := &authorization.Attributes{} + + s.mockMetricsScope.On("StartTimer", metrics.CadenceAuthorizationLatency). + Return(metrics.Stopwatch{}).Once() + s.mockAuthorizer.EXPECT().Authorize(ctx, attr). + Return(authorization.Result{Decision: authorization.DecisionDeny}, errors.New("test")). + Times(1) + s.mockMetricsScope.On("IncCounter", metrics.CadenceErrAuthorizeFailedCounter).Once() + + res, err := s.handler.isAuthorized(ctx, attr, s.mockMetricsScope) + s.False(res) + s.Error(err) +} + +func (s *accessControlledHandlerSuite) TestIsAuthorized_Unauthorized() { + ctx := context.Background() + attr := &authorization.Attributes{} + + s.mockMetricsScope.On("StartTimer", metrics.CadenceAuthorizationLatency). + Return(metrics.Stopwatch{}).Once() + s.mockAuthorizer.EXPECT().Authorize(ctx, attr). + Return(authorization.Result{Decision: authorization.DecisionDeny}, nil). + Times(1) + s.mockMetricsScope.On("IncCounter", metrics.CadenceErrUnauthorizedCounter).Once() + + res, err := s.handler.isAuthorized(ctx, attr, s.mockMetricsScope) + s.False(res) + s.NoError(err) +} diff --git a/service/frontend/dcRedirectionHandler_test.go b/service/frontend/dcRedirectionHandler_test.go index f7b256f37b9..10ef193f508 100644 --- a/service/frontend/dcRedirectionHandler_test.go +++ b/service/frontend/dcRedirectionHandler_test.go @@ -366,7 +366,7 @@ func (s *dcRedirectionHandlerSuite) TestQueryWorkflow() { s.domainName, apiName, mock.Anything).Return(nil).Times(1) req := &shared.QueryWorkflowRequest{ - Domain: common.StringPtr(s.domainName), + Domain: common.StringPtr(s.domainName), QueryConsistencyLevel: shared.QueryConsistencyLevelStrong.Ptr(), } resp, err := s.handler.QueryWorkflow(context.Background(), req) diff --git a/service/frontend/workflowHandler.go b/service/frontend/workflowHandler.go index 7c1b98b3f5f..66e78c2be02 100644 --- a/service/frontend/workflowHandler.go +++ b/service/frontend/workflowHandler.go @@ -3412,12 +3412,7 @@ func (wh *WorkflowHandler) startRequestProfile(scope int) (metrics.Scope, metric // startRequestProfileWithDomain initiates recording of request metrics and returns a domain tagged scope func (wh *WorkflowHandler) startRequestProfileWithDomain(scope int, d domainGetter) (metrics.Scope, metrics.Stopwatch) { - var metricsScope metrics.Scope - if d != nil { - metricsScope = wh.GetMetricsClient().Scope(scope).Tagged(metrics.DomainTag(d.GetDomain())) - } else { - metricsScope = wh.GetMetricsClient().Scope(scope).Tagged(metrics.DomainUnknownTag()) - } + metricsScope := getMetricsScopeWithDomain(scope, d, wh.GetMetricsClient()) sw := metricsScope.StartTimer(metrics.CadenceLatency) metricsScope.IncCounter(metrics.CadenceRequests) return metricsScope, sw