Skip to content

Commit f7d936d

Browse files
authored
[common/log] Unify logger package (#6779)
* [common/log] Unify logger package
1 parent 7e97270 commit f7d936d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+227
-195
lines changed

bench/lib/context.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
"github.com/uber-go/tally"
2525
"go.uber.org/zap"
2626

27-
"github.com/uber/cadence/common/log/loggerimpl"
27+
"github.com/uber/cadence/common/log"
2828
)
2929

3030
const (
@@ -59,7 +59,7 @@ func NewRuntimeContext(cfg *Config) (*RuntimeContext, error) {
5959
return nil, err
6060
}
6161

62-
metricsScope := cfg.Metrics.NewScope(loggerimpl.NewLogger(logger), cfg.Bench.Name)
62+
metricsScope := cfg.Metrics.NewScope(log.NewLogger(logger), cfg.Bench.Name)
6363

6464
if cfg.Cadence.ServiceName == "" {
6565
cfg.Cadence.ServiceName = defaultCadenceServiceName

canary/runner.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ import (
4040
"go.uber.org/zap"
4141
"google.golang.org/grpc/credentials"
4242

43-
"github.com/uber/cadence/common/log/loggerimpl"
43+
"github.com/uber/cadence/common/log"
4444
)
4545

4646
type canaryRunner struct {
@@ -56,7 +56,7 @@ func NewCanaryRunner(cfg *Config) (Runnable, error) {
5656
return nil, fmt.Errorf("failed to create logger: %v", err)
5757
}
5858

59-
metricsScope := cfg.Metrics.NewScope(loggerimpl.NewLogger(logger), "cadence-canary")
59+
metricsScope := cfg.Metrics.NewScope(log.NewLogger(logger), "cadence-canary")
6060

6161
if cfg.Cadence.ServiceName == "" {
6262
cfg.Cadence.ServiceName = CadenceServiceName

cmd/server/cadence/server.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ import (
4949
"github.com/uber/cadence/common/elasticsearch"
5050
"github.com/uber/cadence/common/isolationgroup/isolationgroupapi"
5151
cadencelog "github.com/uber/cadence/common/log"
52-
"github.com/uber/cadence/common/log/loggerimpl"
5352
"github.com/uber/cadence/common/log/tag"
5453
"github.com/uber/cadence/common/membership"
5554
"github.com/uber/cadence/common/messaging/kafka"
@@ -129,7 +128,7 @@ func (s *server) startService() common.Daemon {
129128
if err != nil {
130129
log.Fatal("failed to create the zap logger, err: ", err.Error())
131130
}
132-
params.Logger = loggerimpl.NewLogger(zapLogger).WithTags(tag.Service(params.Name))
131+
params.Logger = cadencelog.NewLogger(zapLogger).WithTags(tag.Service(params.Name))
133132

134133
params.PersistenceConfig = s.cfg.Persistence
135134

cmd/server/cadence/server_test.go

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
package cadence
2525

2626
import (
27-
"log"
2827
"os"
2928
"testing"
3029
"time"
@@ -37,7 +36,9 @@ import (
3736
"github.com/uber/cadence/common"
3837
"github.com/uber/cadence/common/config"
3938
"github.com/uber/cadence/common/dynamicconfig"
40-
"github.com/uber/cadence/common/log/loggerimpl"
39+
"github.com/uber/cadence/common/log"
40+
"github.com/uber/cadence/common/log/tag"
41+
"github.com/uber/cadence/common/log/testlogger"
4142
"github.com/uber/cadence/common/persistence/nosql/nosqlplugin/cassandra/gocql"
4243
"github.com/uber/cadence/common/resource"
4344
"github.com/uber/cadence/common/service"
@@ -51,6 +52,8 @@ import (
5152
type ServerSuite struct {
5253
*require.Assertions
5354
suite.Suite
55+
56+
logger log.Logger
5457
}
5558

5659
func TestServerSuite(t *testing.T) {
@@ -60,6 +63,7 @@ func TestServerSuite(t *testing.T) {
6063

6164
func (s *ServerSuite) SetupTest() {
6265
s.Assertions = require.New(s.T())
66+
s.logger = testlogger.New(s.T())
6367
}
6468

6569
/*
@@ -77,7 +81,7 @@ func (s *ServerSuite) TestServerStartup() {
7781
var cfg config.Config
7882
err := config.Load(env, configDir, zone, &cfg)
7983
if err != nil {
80-
log.Fatal("Config file corrupted.", err)
84+
s.logger.Fatal("Config file corrupted.", tag.Error(err))
8185
}
8286

8387
if os.Getenv("CASSANDRA_SEEDS") == "cassandra" {
@@ -97,11 +101,11 @@ func (s *ServerSuite) TestServerStartup() {
97101
cfg.DynamicConfig.FileBased.Filepath = constructPathIfNeed(rootDir, cfg.DynamicConfig.FileBased.Filepath)
98102

99103
if err := cfg.ValidateAndFillDefaults(); err != nil {
100-
log.Fatalf("config validation failed: %v", err)
104+
s.logger.Fatal("config validation failed", tag.Error(err))
101105
}
102106
// cassandra schema version validation
103107
if err := cassandra.VerifyCompatibleVersion(cfg.Persistence, gocql.All); err != nil {
104-
log.Fatal("cassandra schema version compatibility check failed: ", err)
108+
s.logger.Fatal("cassandra schema version compatibility check failed", tag.Error(err))
105109
}
106110

107111
var daemons []common.Daemon
@@ -128,11 +132,11 @@ func TestSettingGettingZonalIsolationGroupsFromIG(t *testing.T) {
128132
"zone-1", "zone-2",
129133
}, nil)
130134

131-
dc := dynamicconfig.NewCollection(client, loggerimpl.NewNopLogger())
135+
dc := dynamicconfig.NewCollection(client, log.NewNoop())
132136

133137
assert.NotPanics(t, func() {
134138
fn := getFromDynamicConfig(resource.Params{
135-
Logger: loggerimpl.NewNopLogger(),
139+
Logger: log.NewNoop(),
136140
}, dc)
137141
out := fn()
138142
assert.Equal(t, []string{"zone-1", "zone-2"}, out)
@@ -143,11 +147,11 @@ func TestSettingGettingZonalIsolationGroupsFromIGError(t *testing.T) {
143147
ctrl := gomock.NewController(t)
144148
client := dynamicconfig.NewMockClient(ctrl)
145149
client.EXPECT().GetListValue(dynamicconfig.AllIsolationGroups, gomock.Any()).Return(nil, assert.AnError)
146-
dc := dynamicconfig.NewCollection(client, loggerimpl.NewNopLogger())
150+
dc := dynamicconfig.NewCollection(client, log.NewNoop())
147151

148152
assert.NotPanics(t, func() {
149153
getFromDynamicConfig(resource.Params{
150-
Logger: loggerimpl.NewNopLogger(),
154+
Logger: log.NewNoop(),
151155
}, dc)()
152156
})
153157
}

common/cluster/metadata_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import (
3131
"github.com/stretchr/testify/require"
3232

3333
"github.com/uber/cadence/common/config"
34-
"github.com/uber/cadence/common/log/loggerimpl"
34+
"github.com/uber/cadence/common/log"
3535
"github.com/uber/cadence/common/log/testlogger"
3636
"github.com/uber/cadence/common/metrics"
3737
)
@@ -226,7 +226,7 @@ func TestResolvingClusterVersion(t *testing.T) {
226226
initialFailoverVersionC2: clusterName2,
227227
},
228228
metrics: metrics.NewNoopMetricsClient().Scope(0),
229-
log: loggerimpl.NewNopLogger(),
229+
log: log.NewNoop(),
230230
}
231231

232232
for name, td := range tests {
@@ -290,7 +290,7 @@ func TestIsPartOfTheSameCluster(t *testing.T) {
290290
initialFailoverVersionC1: clusterName1,
291291
initialFailoverVersionC2: clusterName2,
292292
},
293-
log: loggerimpl.NewNopLogger(),
293+
log: log.NewNoop(),
294294
metrics: metrics.NewNoopMetricsClient().Scope(0),
295295
}
296296

@@ -532,7 +532,7 @@ func TestIsPartOfTheSameClusterAPIFixing(t *testing.T) {
532532
},
533533
useNewFailoverVersionOverride: func(domain string) bool { return false },
534534
metrics: metrics.NewNoopMetricsClient().Scope(0),
535-
log: loggerimpl.NewNopLogger(),
535+
log: log.NewNoop(),
536536
}
537537

538538
for i := range tests {
@@ -774,7 +774,7 @@ func TestClusterNameForFailoverVersion(t *testing.T) {
774774
},
775775
metrics: metrics.NewNoopMetricsClient().Scope(0),
776776
useNewFailoverVersionOverride: func(domain string) bool { return false },
777-
log: loggerimpl.NewNopLogger(),
777+
log: log.NewNoop(),
778778
}
779779

780780
for i := range tests {
@@ -811,7 +811,7 @@ func TestServerResolution(t *testing.T) {
811811
},
812812
metrics: metrics.NewNoopMetricsClient().Scope(0),
813813
useNewFailoverVersionOverride: func(domain string) bool { return domain == domainToMigrate },
814-
log: loggerimpl.NewNopLogger(),
814+
log: log.NewNoop(),
815815
}
816816

817817
err := quick.Check(func(currentFOVersion int64, migrateDomain bool) bool {
@@ -876,7 +876,7 @@ func TestNoChangesInUnmigratedState(t *testing.T) {
876876
},
877877
metrics: metrics.NewNoopMetricsClient().Scope(0),
878878
useNewFailoverVersionOverride: func(domain string) bool { return false },
879-
log: loggerimpl.NewNopLogger(),
879+
log: log.NewNoop(),
880880
}
881881

882882
err := quick.CheckEqual(func(currVersion int64) int64 {
@@ -920,7 +920,7 @@ func TestFailoverVersionResolution(t *testing.T) {
920920
},
921921
metrics: metrics.NewNoopMetricsClient().Scope(0),
922922
useNewFailoverVersionOverride: func(domain string) bool { return false },
923-
log: loggerimpl.NewNopLogger(),
923+
log: log.NewNoop(),
924924
}
925925

926926
tests := map[string]struct {

common/cluster/metadata_test_utils.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ package cluster
2222

2323
import (
2424
"github.com/uber/cadence/common/config"
25-
"github.com/uber/cadence/common/log/loggerimpl"
25+
"github.com/uber/cadence/common/log"
2626
commonMetrics "github.com/uber/cadence/common/metrics"
2727
"github.com/uber/cadence/common/service"
2828
)
@@ -96,7 +96,7 @@ var (
9696
TestAllClusterInfo,
9797
func(d string) bool { return false },
9898
commonMetrics.NewNoopMetricsClient(),
99-
loggerimpl.NewNopLogger(),
99+
log.NewNoop(),
100100
)
101101

102102
// TestPassiveClusterMetadata is metadata for a passive cluster
@@ -107,7 +107,7 @@ var (
107107
TestAllClusterInfo,
108108
func(d string) bool { return false },
109109
commonMetrics.NewNoopMetricsClient(),
110-
loggerimpl.NewNopLogger(),
110+
log.NewNoop(),
111111
)
112112
)
113113

@@ -125,6 +125,6 @@ func GetTestClusterMetadata(isPrimaryCluster bool) Metadata {
125125
TestAllClusterInfo,
126126
func(d string) bool { return false },
127127
commonMetrics.NewNoopMetricsClient(),
128-
loggerimpl.NewNopLogger(),
128+
log.NewNoop(),
129129
)
130130
}

common/quotas/dynamicratelimiterfactory.go renamed to common/dynamicconfig/quotas/dynamicratelimiterfactory.go

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,16 @@
2020
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
2121
// SOFTWARE.
2222

23-
//go:generate mockgen -package=$GOPACKAGE -destination=limiterfactory_mock.go github.com/uber/cadence/common/quotas LimiterFactory
24-
2523
package quotas
2624

2725
import (
2826
"github.com/uber/cadence/common/dynamicconfig"
27+
"github.com/uber/cadence/common/quotas"
2928
)
3029

31-
// LimiterFactory is used to create a Limiter for a given domain
32-
type LimiterFactory interface {
33-
// GetLimiter returns a new Limiter for the given domain
34-
GetLimiter(domain string) Limiter
35-
}
36-
3730
// NewSimpleDynamicRateLimiterFactory creates a new LimiterFactory which creates
3831
// a new DynamicRateLimiter for each domain, the RPS for the DynamicRateLimiter is given by the dynamic config
39-
func NewSimpleDynamicRateLimiterFactory(rps dynamicconfig.IntPropertyFnWithDomainFilter) LimiterFactory {
32+
func NewSimpleDynamicRateLimiterFactory(rps dynamicconfig.IntPropertyFnWithDomainFilter) quotas.LimiterFactory {
4033
return dynamicRateLimiterFactory{
4134
rps: rps,
4235
}
@@ -47,6 +40,6 @@ type dynamicRateLimiterFactory struct {
4740
}
4841

4942
// GetLimiter returns a new Limiter for the given domain
50-
func (f dynamicRateLimiterFactory) GetLimiter(domain string) Limiter {
51-
return NewDynamicRateLimiter(func() float64 { return float64(f.rps(domain)) })
43+
func (f dynamicRateLimiterFactory) GetLimiter(domain string) quotas.Limiter {
44+
return quotas.NewDynamicRateLimiter(func() float64 { return float64(f.rps(domain)) })
5245
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// The MIT License (MIT)
2+
3+
// Copyright (c) 2017-2020 Uber Technologies Inc.
4+
5+
// Permission is hereby granted, free of charge, to any person obtaining a copy
6+
// of this software and associated documentation files (the "Software"), to deal
7+
// in the Software without restriction, including without limitation the rights
8+
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
9+
// copies of the Software, and to permit persons to whom the Software is
10+
// furnished to do so, subject to the following conditions:
11+
//
12+
// The above copyright notice and this permission notice shall be included in all
13+
// copies or substantial portions of the Software.
14+
//
15+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
16+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
17+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
18+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
19+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
20+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
21+
// SOFTWARE.
22+
23+
package quotas
24+
25+
import (
26+
"testing"
27+
28+
"github.com/stretchr/testify/assert"
29+
"golang.org/x/time/rate"
30+
)
31+
32+
func TestNewSimpleDynamicRateLimiterFactory(t *testing.T) {
33+
const _testDomain = "test-domain"
34+
35+
factory := NewSimpleDynamicRateLimiterFactory(func(domain string) int {
36+
assert.Equal(t, _testDomain, domain)
37+
return 100
38+
})
39+
40+
limiter := factory.GetLimiter(_testDomain)
41+
42+
assert.Equal(t, rate.Limit(100), limiter.Limit())
43+
}

common/quotas/fallbackdynamicratelimiterfactory.go renamed to common/dynamicconfig/quotas/fallbackdynamicratelimiterfactory.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,18 @@
2222

2323
package quotas
2424

25-
import "github.com/uber/cadence/common/dynamicconfig"
25+
import (
26+
"github.com/uber/cadence/common/dynamicconfig"
27+
"github.com/uber/cadence/common/quotas"
28+
)
2629

27-
// LimiterFactory is used to create a Limiter for a given domain
30+
// NewFallbackDynamicRateLimiterFactory is used to create a Limiter for a given domain
2831
// the created Limiter will use the primary dynamic config if it is set
2932
// otherwise it will use the secondary dynamic config
3033
func NewFallbackDynamicRateLimiterFactory(
3134
primary dynamicconfig.IntPropertyFnWithDomainFilter,
3235
secondary dynamicconfig.IntPropertyFn,
33-
) LimiterFactory {
36+
) quotas.LimiterFactory {
3437
return fallbackDynamicRateLimiterFactory{
3538
primary: primary,
3639
secondary: secondary,
@@ -44,8 +47,8 @@ type fallbackDynamicRateLimiterFactory struct {
4447
}
4548

4649
// GetLimiter returns a new Limiter for the given domain
47-
func (f fallbackDynamicRateLimiterFactory) GetLimiter(domain string) Limiter {
48-
return NewDynamicRateLimiter(func() float64 {
50+
func (f fallbackDynamicRateLimiterFactory) GetLimiter(domain string) quotas.Limiter {
51+
return quotas.NewDynamicRateLimiter(func() float64 {
4952
return limitWithFallback(
5053
float64(f.primary(domain)),
5154
float64(f.secondary()))

common/isolationgroup/defaultisolationgroupstate/state_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import (
3434
"github.com/uber/cadence/common/cache"
3535
"github.com/uber/cadence/common/dynamicconfig"
3636
"github.com/uber/cadence/common/isolationgroup/isolationgroupapi"
37-
"github.com/uber/cadence/common/log/loggerimpl"
37+
"github.com/uber/cadence/common/log"
3838
"github.com/uber/cadence/common/log/testlogger"
3939
"github.com/uber/cadence/common/metrics"
4040
"github.com/uber/cadence/common/persistence"
@@ -326,7 +326,7 @@ func TestNewDefaultIsolationGroupStateWatcherWithConfigStoreClient(t *testing.T)
326326
client := metrics.NewNoopMetricsClient()
327327
ig := func() []string { return nil }
328328
NewDefaultIsolationGroupStateWatcherWithConfigStoreClient(
329-
loggerimpl.NewNopLogger(),
329+
log.NewNoop(),
330330
dc,
331331
domainCache,
332332
nil,

0 commit comments

Comments
 (0)