diff --git a/service/sharddistributor/store/wrappers/metered/base.go b/service/sharddistributor/store/wrappers/metered/base.go index 85ec58d1fe9..ffe499c7f19 100644 --- a/service/sharddistributor/store/wrappers/metered/base.go +++ b/service/sharddistributor/store/wrappers/metered/base.go @@ -27,7 +27,6 @@ import ( "github.com/uber/cadence/common/clock" "github.com/uber/cadence/common/log" - "github.com/uber/cadence/common/log/tag" "github.com/uber/cadence/common/metrics" "github.com/uber/cadence/service/sharddistributor/store" ) @@ -38,18 +37,11 @@ type base struct { timeSource clock.TimeSource } -func (p *base) updateErrorMetricPerNamespace(scope metrics.ScopeIdx, err error, scopeWithNamespaceTags metrics.Scope, logger log.Logger) { - logger = logger.Helper() - - switch { - case errors.Is(err, store.ErrExecutorNotFound): +func (p *base) updateErrorMetricPerNamespace(err error, scopeWithNamespaceTags metrics.Scope) { + if errors.Is(err, store.ErrExecutorNotFound) { scopeWithNamespaceTags.IncCounter(metrics.ShardDistributorStoreExecutorNotFound) - logger.Error("Executor not found.", tag.Error(err), tag.MetricScope(int(scope))) - case errors.Is(err, store.ErrShardNotFound): - // this is expected, so we don't want to log it - default: - logger.Error("Store failed with internal error.", tag.Error(err), tag.MetricScope(int(scope))) // int??? } + scopeWithNamespaceTags.IncCounter(metrics.ShardDistributorStoreFailuresPerNamespace) } @@ -62,9 +54,8 @@ func (p *base) call(scope metrics.ScopeIdx, op func() error, tags ...metrics.Tag duration := p.timeSource.Since(before) metricsScope.RecordHistogramDuration(metrics.ShardDistributorStoreLatencyHistogramPerNamespace, duration) - logger := p.logger.Helper() if err != nil { - p.updateErrorMetricPerNamespace(scope, err, metricsScope, logger) + p.updateErrorMetricPerNamespace(err, metricsScope) } return err } diff --git a/service/sharddistributor/store/wrappers/metered/metered_test.go b/service/sharddistributor/store/wrappers/metered/metered_test.go index 5874cc736f2..a7e1564becf 100644 --- a/service/sharddistributor/store/wrappers/metered/metered_test.go +++ b/service/sharddistributor/store/wrappers/metered/metered_test.go @@ -11,7 +11,6 @@ import ( "github.com/uber/cadence/common/clock" "github.com/uber/cadence/common/log" - "github.com/uber/cadence/common/log/tag" "github.com/uber/cadence/common/metrics" "github.com/uber/cadence/common/types" "github.com/uber/cadence/service/sharddistributor/store" @@ -31,33 +30,19 @@ func TestMeteredStore_GetHeartbeat(t *testing.T) { } tests := []struct { - name string - setupMocks func(logger *log.MockLogger) - error error + name string + error error }{ { - name: "Success", - setupMocks: func(logger *log.MockLogger) {}, - error: nil, + name: "Success", + error: nil, }, { - name: "NotFound", - setupMocks: func(logger *log.MockLogger) { - logger.EXPECT().Error( - "Executor not found.", - []tag.Tag{tag.Error(store.ErrExecutorNotFound), tag.MetricScope(int(metrics.ShardDistributorStoreGetHeartbeatScope))}, - ).Times(1) - }, + name: "NotFound", error: store.ErrExecutorNotFound, }, { - name: "Failure", - setupMocks: func(logger *log.MockLogger) { - logger.EXPECT().Error( - "Store failed with internal error.", - []tag.Tag{tag.Error(&types.InternalServiceError{}), tag.MetricScope(int(metrics.ShardDistributorStoreGetHeartbeatScope))}, - ).Times(1) - }, + name: "Failure", error: &types.InternalServiceError{}, }, } @@ -79,7 +64,6 @@ func TestMeteredStore_GetHeartbeat(t *testing.T) { mockLogger.EXPECT().Helper().Return(mockLogger).AnyTimes() wrapped := NewStore(mockHandler, metricsClient, mockLogger, timeSource).(*meteredStore) - tt.setupMocks(mockLogger) gotHeartbeat, gotAssignedState, err := wrapped.GetHeartbeat(context.Background(), _testNamespace, _testExecutorID)